From f917d11a38e66518f497c5c940c2e52a5d63f9fa Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 8 Sep 2026 16:09:46 +0000 Subject: [PATCH] fix(spec): refuse a whitespace-only `reference` on lookup/master_detail MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The #13632 refinement spelled its emptiness test as `reference === ''`, so a whitespace-only target passed a door whose whole purpose is to name an object. Measured on the built artifact: absent and `''` were refused while `' '` and `'\t\n'` were accepted, at both the field and the document level. Apply the test to the trimmed value, so a blank target joins absent and `''` under the same `custom` issue, the same `reference` path and the same message. The notion of blank is `.trim()` — the same one `EvaluatedExpressionSchema` applies to `source`, not a third one. Trimming is for the TEST only: a target with surrounding whitespace is authored and is still stored as written, and a non-string still answers `invalid_type` from the base schema. Both boundaries are pinned. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_016N6xmWt5hYm94ffVEwGH8x --- .changeset/field-reference-non-blank.md | 51 ++++++++++++++++++ packages/spec/src/data/field.test.ts | 72 +++++++++++++++++++++++++ packages/spec/src/data/field.zod.ts | 19 ++++++- 3 files changed, 140 insertions(+), 2 deletions(-) create mode 100644 .changeset/field-reference-non-blank.md diff --git a/.changeset/field-reference-non-blank.md b/.changeset/field-reference-non-blank.md new file mode 100644 index 0000000000..e928ece5e0 --- /dev/null +++ b/.changeset/field-reference-non-blank.md @@ -0,0 +1,51 @@ +--- +"@objectstack/spec": minor +--- + +fix(spec): `FieldSchema` refuses a WHITESPACE-ONLY `reference` on `lookup` / `master_detail` + +**BREAKING** accept-set narrowing on `FieldSchema`, shipped as `minor` under the +repo's launch-window convention for breaking changes — the same grade the nearest +tightening precedents shipped with, including #13632, the narrowing this one +finishes. + +#13632 closed the declared-but-unenforced gap on `FieldSchema.reference` in 17.3.0, +but spelled its emptiness test as an equality against `''`, so a whitespace-only +target (`reference: ' '`) passed a door whose whole purpose is to name an object. +Measured on the built artifact before this change: absent and `''` were refused, +`' '` and `'\t\n'` were **accepted**, at both the field level (`FieldSchema`) and +the document level (`ObjectSchema`), on `lookup` and `master_detail` alike. + +A blank target names no object either. The declared grammar for an object name is +`/^[a-z_][a-z0-9_]*$/` (`ObjectSchema`'s own `fields` key schema), so no +whitespace-bearing string can ever resolve to one, and all three consequences the +existing refusal message lists hold verbatim for `' '`: the record picker has no +object to query, `$expand` has nothing to resolve, and no relationship index can be +built. It is also the state a cleared target picker emits — `''` and `' '` are one +authoring gesture that was getting opposite verdicts. + +What newly gets rejected: `type: 'lookup'` or `type: 'master_detail'` whose +`reference` is present but consists only of whitespace. It joins absent and `''` +under the same `custom` issue, on the same `reference` path, with the same +prescriptive message — no new message and no new error shape. The notion of blank +is `.trim()`, the same one `EvaluatedExpressionSchema` applies to `source`, not a +third one. + +Everything else is untouched. Trimming is applied to the TEST only, never to the +stored value: a target with surrounding whitespace (`' company '`) is still accepted +and still round-trips byte-identically. A non-string `reference` still answers +`invalid_type` from the base schema, not the custom message — that distinction is +deliberate and pinned. Non-relationship types never carried the requirement, and the +`Field.lookup()` / `Field.masterDetail()` helpers take the target as their first +positional argument, so helper-authored fields cannot produce this shape. + +The measured population of affected authored sources is zero: one repo-wide census +over all tracked files found a single whitespace-only `reference` in the tree, an +objectql test fixture cast past Zod on the documented `registerObject` path that +skips schema validation by design — it does not reach this door, and it is green +after the change. The census and its positive controls are recorded on the PR. +Downstream, objectui's two metadata writers already refuse this shape with +`reference.trim() !== ''`; upstream trimming turns their declared divergence into +contract-following, and that note can now be retired. + + diff --git a/packages/spec/src/data/field.test.ts b/packages/spec/src/data/field.test.ts index ddc0bef4ca..de2ee1b18c 100644 --- a/packages/spec/src/data/field.test.ts +++ b/packages/spec/src/data/field.test.ts @@ -2278,6 +2278,78 @@ describe('Relationship target — `reference` required on lookup/master_detail ( }, ); + // [#16126] A whitespace-only target is the same hole a third way: it names + // no object either (no whitespace-bearing string can match the declared + // object-name grammar), and it is what a cleared target picker emits when + // the value round-trips through an input. The notion of blank is `.trim()`, + // the same one `EvaluatedExpressionSchema` applies to `source`. + + it.each(['lookup', 'master_detail'] as const)( + 'refuses a %s whose reference is whitespace-only — same issue, same path, same message as `\'\'`', + (type) => { + const result = FieldSchema.safeParse({ + name: 'company_id', + label: 'Company', + type, + reference: ' ', + }); + expect(result.success).toBe(false); + const issue = result.error!.issues.find((i) => i.path.join('.') === 'reference'); + expect(issue).toBeDefined(); + expect(issue!.code).toBe('custom'); + expect(issue!.message).toMatch(/non-empty `reference`/); + expect(issue!.message).toMatch(/target object/); + }, + ); + + it.each(['\t', '\n', ' \t\n '] as const)( + 'refuses a lookup whose reference is only whitespace (%j) — not just the space character', + (reference) => { + const result = FieldSchema.safeParse({ + name: 'company_id', label: 'Company', type: 'lookup', reference, + }); + expect(result.success).toBe(false); + expect( + result.error!.issues.find((i) => i.path.join('.') === 'reference')?.code, + ).toBe('custom'); + }, + ); + + it('refuses a whitespace-only reference at the DOCUMENT level too, located at the field', () => { + const result = ObjectSchema.safeParse({ + name: 'acct_note', + label: 'Note', + fields: { rel: { name: 'rel', label: 'Rel', type: 'lookup', reference: ' ' } }, + }); + expect(result.success).toBe(false); + const issue = result.error!.issues.find( + (i) => i.path.join('.') === 'fields.rel.reference', + ); + expect(issue).toBeDefined(); + expect(issue!.code).toBe('custom'); + }); + + it.each([42, null, {}] as const)( + 'keeps a non-string reference (%j) answering `invalid_type`, not the custom message', + (reference) => { + const result = FieldSchema.safeParse({ + name: 'company_id', label: 'Company', type: 'lookup', reference, + }); + expect(result.success).toBe(false); + const issue = result.error!.issues.find((i) => i.path.join('.') === 'reference'); + expect(issue!.code).toBe('invalid_type'); + expect(issue!.message).not.toMatch(/non-empty `reference`/); + }, + ); + + it('trims only to TEST — a name with surrounding whitespace is authored and is stored as written', () => { + const result = FieldSchema.safeParse({ + name: 'company_id', label: 'Company', type: 'lookup', reference: ' company ', + }); + expect(result.success).toBe(true); + if (result.success) expect(result.data.reference).toBe(' company '); + }); + it.each(['lookup', 'master_detail'] as const)( 'accepts a %s with a non-empty reference (positive control: the check refuses only the hole)', (type) => { diff --git a/packages/spec/src/data/field.zod.ts b/packages/spec/src/data/field.zod.ts index 9f90c8e11d..818caeaea8 100644 --- a/packages/spec/src/data/field.zod.ts +++ b/packages/spec/src/data/field.zod.ts @@ -1129,7 +1129,8 @@ export const FieldSchema = lazySchema(() => { * Used by `lookup` and `master_detail` field types to define cross-object references. * The `reference` property is **required** for these types — it identifies the target * object whose records this field links to, and the superRefine below enforces it: - * a `lookup` / `master_detail` whose `reference` is missing or empty is refused at + * a `lookup` / `master_detail` whose `reference` is missing, empty, or + * whitespace-only is refused at * parse time. The engine uses `reference` during $expand * post-processing to resolve foreign key IDs into full related objects via batch queries. * @@ -1822,9 +1823,23 @@ export const FieldSchema = lazySchema(() => { // measured as accepted before this check). `Field.lookup()` / // `Field.masterDetail()` take the target as their first positional // argument, so helper-authored fields cannot miss it. + // + // [#16126] The emptiness test is applied to the TRIMMED value, so a + // whitespace-only `reference` joins `undefined` and `''` under this one + // issue and this one message. It names no object either: the declared + // grammar for an object name is `/^[a-z_][a-z0-9_]*$/` (`ObjectSchema`'s + // own `fields` key schema), so no whitespace-bearing string can ever + // resolve to one, and all three consequences the message lists hold + // verbatim for `' '`. It is also the state a cleared target picker + // emits: `''` and `' '` are one authoring gesture that was getting + // opposite verdicts. The notion of blank is `.trim()` — the same one + // `EvaluatedExpressionSchema` applies to `source`, not a third one. + // Trimming is for the TEST only: a value with surrounding whitespace is + // authored and is still stored as written, and a non-string still + // answers `invalid_type` from the base schema before this runs. if ( (field.type === 'lookup' || field.type === 'master_detail') && - (field.reference === undefined || field.reference === '') + (field.reference === undefined || field.reference.trim() === '') ) { ctx.addIssue({ code: 'custom',