fix(spec): isValueDomainMember refuses an off-vocabulary domain instead of failing open

.changeset/value-domain-membership-off-vocabulary-refusal.md

@@ -0,0 +1,54 @@
+---
+'@objectstack/spec': patch
+---
+
+fix(spec): `isValueDomainMember` refuses an off-vocabulary domain instead of failing OPEN on `Object.prototype` names
+
+`DOMAIN_MEMBERSHIP` is an object literal, so it inherits `Object.prototype`, and
+`isValueDomainMember` indexed it with no own-property guard. Measured against the
+built artifact (`dist/shared/index.mjs`) on the repo's Node 22 baseline (v22.22.2),
+an off-vocabulary `domain` did one of two wrong things — and one of them was a
+membership FALSE POSITIVE out of a predicate whose whole job is to refuse
+non-members:
+
+| `domain` | before | after |
+|:--|:--|:--|
+| `iana_time_zone` (in vocabulary) | `true` for `UTC` | `true` for `UTC` — unmoved |
+| `toString` | `'[object Object]'` — a truthy **string** | `false` |
+| `valueOf` | a truthy **object** | `false` |
+| `constructor` | a truthy **object** | `false` |
+| `__proto__` | threw a `TypeError` | `false` |
+| `nope`, `''` | threw a `TypeError` | `false` |
+
+**Why it is reachable.** "Unreachable in-repo" is not "unreachable". The parameter
+is typed `ValueDomain` and every in-repo call site names a member, but
+`isValueDomainMember` is **published** on `@objectstack/spec/shared` (it is in
+`packages/spec/api-surface/shared.json`). A plain-JS consumer, or any caller
+handing over a domain string read from **metadata** rather than written in source,
+reaches it with no type checking at all — and metadata-sourced strings are exactly
+where `constructor` and `toString` show up.
+
+**This narrows and widens nothing, measured rather than asserted.** Every accepted
+`domain` is an own key of the record, so no value that was accepted before is
+refused now; the three real domains answer from their own definitions, unmoved.
+The change is one `Object.prototype.hasOwnProperty.call` guard — the same spelling
+the `iso_4217_currency` definition in the same module already uses — returning
+`false` for a domain that is not an own key. A **null-prototype record** was the
+other shape available and was not taken: it converts the truthy answers into
+throws rather than into `false`, and it costs the `Readonly<Record<ValueDomain, …>>`
+annotation that makes a vocabulary member added without a definition fail to
+compile.
+
+**Unknown domain answers `false`; it does not throw.** `false` is the narrowing
+reading — it refuses more and accepts nothing new — whereas a thrown refusal would
+change published behaviour for callers who today receive a truthy value. This is
+the same third branch a sister ruling settled for the same defect family: list
+reject / own-member value / prototype-resolvable ⇒ reject.
+
+The pin that existed did not cover this, and the fix is as much about its
+POPULATION as about the guard: the totality pin asserted the return `typeof` was
+`boolean` but iterated `ValueDomainSchema.options` **only** — exactly the domains
+that behave. The new pins put `toString`, `valueOf`, `constructor`,
+`hasOwnProperty`, `isPrototypeOf`, `propertyIsEnumerable`, `__proto__` and plainly
+absent words into the population, and a third pin holds that population honest by
+asserting every one of them is still outside the vocabulary.

packages/spec/src/shared/value-domain.test.ts

@@ -19,6 +19,7 @@ import {
   ValueDomainSchema,
   ISO_3166_ALPHA2_CODES,
   isValueDomainMember,
+  type ValueDomain,
 } from './value-domain.zod';
 import { SpecifierValueDomainSchema } from '../system/settings-manifest.zod';
 import { CURRENCY_FRACTION_DIGITS } from '../data/currency-fraction-digits';
@@ -168,9 +169,100 @@ describe('isValueDomainMember — iso_3166_alpha2 is the explicit 249-code list'
 
 describe('isValueDomainMember — every vocabulary member has a definition', () => {
   it('answers a boolean for each member, never throws, never returns undefined', () => {
+    // ⚠ The POPULATION here is `ValueDomainSchema.options` — exactly the
+    // domains that behave. That is the right population for THIS claim (every
+    // member has a definition), and the wrong one for "an unknown domain is
+    // refused": the describe below carries that claim over the population this
+    // loop cannot reach.
     for (const domain of ValueDomainSchema.options) {
       expect(typeof isValueDomainMember(domain, 'definitely-not-a-member')).toBe('boolean');
       expect(isValueDomainMember(domain, 'definitely-not-a-member')).toBe(false);
     }
   });
 });
+
+describe('isValueDomainMember — an OFF-vocabulary domain is refused, never answered truthy', () => {
+  /**
+   * The published contract, exercised the way a consumer actually reaches it.
+   * `isValueDomainMember` is in `packages/spec/api-surface/shared.json`, so
+   * "unreachable in-repo" is not "unreachable": a plain-JS consumer, or any
+   * caller handing over a domain string read from METADATA rather than written
+   * in source, arrives with zero type checking — and metadata-sourced strings
+   * are exactly where `constructor` and `toString` show up. The cast is that
+   * caller, not a way around the type.
+   */
+  const untyped = (domain: string, value: string): unknown =>
+    isValueDomainMember(domain as ValueDomain, value);
+
+  /**
+   * Domain words that are NOT in the vocabulary, grouped by what each one did
+   * before the own-property guard. `DOMAIN_MEMBERSHIP` is an object literal, so
+   * it inherits `Object.prototype`.
+   */
+  const PROTOTYPE_RESOLVABLE = [
+    // Answered TRUTHY — the membership false positives, the reason this is a
+    // bug and not a tidy-up: `toString` gave the string '[object Object]',
+    // `valueOf` and `constructor` gave objects.
+    'toString',
+    'valueOf',
+    'constructor',
+    // Answered a boolean `false` by accident (`hasOwnProperty` called with
+    // `DOMAIN_MEMBERSHIP` as its receiver), which is why a `typeof` assertion
+    // alone is not enough to catch this family.
+    'hasOwnProperty',
+    'isPrototypeOf',
+    'propertyIsEnumerable',
+    // Resolved to `Object.prototype` itself — not callable, so it THREW a
+    // TypeError. `false` now, like every other non-member.
+    '__proto__',
+  ];
+
+  /** No own key and no prototype member either: these threw a TypeError too. */
+  const PLAINLY_ABSENT = ['nope', '', 'iana_timezone', 'iso_8601_date', 'bcp47_locale', 'ZZ'];
+
+  const OFF_VOCABULARY = [...PROTOTYPE_RESOLVABLE, ...PLAINLY_ABSENT];
+
+  // Values spanning all three real domains plus junk, so a leaked definition
+  // would be caught whichever domain it leaked from.
+  const VALUES = ['UTC', 'USD', 'US', '', 'definitely-not-a-member'];
+
+  it('answers exactly `false` for a domain naming an Object.prototype member', () => {
+    for (const domain of PROTOTYPE_RESOLVABLE) {
+      for (const value of VALUES) {
+        const answer = untyped(domain, value);
+        const at = `${JSON.stringify(domain)} / ${JSON.stringify(value)}`;
+        expect(typeof answer, at).toBe('boolean');
+        expect(answer, at).toBe(false);
+      }
+    }
+  });
+
+  it('answers exactly `false` for a plainly absent domain, where it used to throw', () => {
+    for (const domain of PLAINLY_ABSENT) {
+      for (const value of VALUES) {
+        const answer = untyped(domain, value);
+        const at = `${JSON.stringify(domain)} / ${JSON.stringify(value)}`;
+        expect(typeof answer, at).toBe('boolean');
+        expect(answer, at).toBe(false);
+      }
+    }
+  });
+
+  it('holds this population HONEST — every word above is outside the vocabulary', () => {
+    // Without this, a word promoted into `ValueDomainSchema` would leave the
+    // two pins above asserting `false` for a legal domain, and they would go on
+    // passing while meaning the opposite of what they say.
+    for (const domain of OFF_VOCABULARY) {
+      expect(ValueDomainSchema.safeParse(domain).success, domain).toBe(false);
+      expect(ValueDomainSchema.options as readonly string[], domain).not.toContain(domain);
+    }
+  });
+
+  it('still answers the three real members from their own definitions', () => {
+    // The narrowing must stop at the vocabulary edge: the guard refuses more
+    // and accepts nothing new, so every in-vocabulary answer is unmoved.
+    expect(isValueDomainMember('iana_time_zone', 'UTC')).toBe(true);
+    expect(isValueDomainMember('iso_4217_currency', 'USD')).toBe(true);
+    expect(isValueDomainMember('iso_3166_alpha2', 'US')).toBe(true);
+  });
+});

packages/spec/src/shared/value-domain.zod.ts

@@ -171,7 +171,39 @@ const DOMAIN_MEMBERSHIP: Readonly<Record<ValueDomain, (value: string) => boolean
  * two code domains are exact uppercase). Element-wise iteration over a
  * multi-value carrier, and the prose a refusal message needs, are the
  * caller's: this function answers membership and nothing else.
+ *
+ * Total over the vocabulary AND closed outside it: a `domain` that is not a
+ * member — including one that names an `Object.prototype` member such as
+ * `toString`, `valueOf`, `constructor` or `__proto__` — answers `false`. It
+ * never throws and never answers a non-boolean, so a caller reaching this
+ * published export from plain JS or from metadata cannot get a membership
+ * false positive out of an unknown domain word. See the guard's own comment.
  */
 export function isValueDomainMember(domain: ValueDomain, value: string): boolean {
+  // ⛔ The own-property guard is load-bearing, not defensive noise. `domain` is
+  // typed, but this function is PUBLISHED (`api-surface/shared.json`), so a
+  // plain-JS consumer — or any caller handing over a domain string read from
+  // METADATA rather than written in source — arrives with no type checking at
+  // all, and metadata-sourced strings are exactly where `constructor` and
+  // `toString` show up. `DOMAIN_MEMBERSHIP` is an object literal, so it
+  // inherits `Object.prototype` and a bare `DOMAIN_MEMBERSHIP[domain]` resolves
+  // a prototype member for such a word: measured on the baseline, `toString`
+  // answered `'[object Object]'` (a truthy STRING), `valueOf` and `constructor`
+  // answered truthy OBJECTS, and `__proto__`, `nope` and `''` threw a
+  // `TypeError` off a non-callable. A predicate whose whole job is to refuse
+  // non-members therefore failed OPEN on three of them.
+  //
+  // The guard collapses every off-vocabulary domain onto one answer — `false`,
+  // never truthy and never a throw — whether it is prototype-resolvable or
+  // plainly absent. It narrows: nothing that was accepted before is refused
+  // now, because every accepted `domain` is an own key. It is the same
+  // spelling the `iso_4217_currency` definition above already uses.
+  //
+  // ⛔ Not `Object.create(null)` for the record: the null prototype would turn
+  // the truthy answers into throws rather than into `false`, and it would cost
+  // the `Readonly<Record<ValueDomain, …>>` annotation that makes a vocabulary
+  // member added without a definition fail to COMPILE — the guarantee the
+  // record's own doc comment above exists to state.
+  if (!Object.prototype.hasOwnProperty.call(DOMAIN_MEMBERSHIP, domain)) return false;
   return DOMAIN_MEMBERSHIP[domain](value);
 }