diff --git a/.changeset/17818-prototype-fallthrough-lookups.md b/.changeset/17818-prototype-fallthrough-lookups.md new file mode 100644 index 0000000000..b7832926a4 --- /dev/null +++ b/.changeset/17818-prototype-fallthrough-lookups.md @@ -0,0 +1,51 @@ +--- +'@objectstack/spec': minor +--- + +fix(spec): four lookup folds no longer hand out `Object.prototype` members for an off-vocabulary key (#17818) + +`normalizeFilterOperator` (`/ui`), `resolveDiscoveryEnvironment` (`/api`), and +`pluralToSingular` / `singularToPlural` (`/meta-spelling`, re-exported from +`/shared`) each read a module-level lookup table with a runtime key through a +bare index. Every one of those tables is an ordinary object, so a key that is +not in the vocabulary resolved a member of `Object.prototype` instead of +falling through — and the `?? fallback` each function already writes never +fired, because the inherited member is truthy. + +Measured on Node v22.22.2, before and after — each fold evaluated at this +change's implementation and again at its merge base, against the TypeScript +sources that the build and the test run both consume: + +| call | before | after | +|:--|:--|:--| +| `normalizeFilterOperator('constructor')` | the `Object` function | `'constructor'` | +| `normalizeFilterOperator('toString')` | `Object.prototype.toString` | `'toString'` | +| `normalizeFilterOperator('valueOf')` | `Object.prototype.valueOf` | `'valueOf'` | +| `normalizeFilterOperator('__proto__')` | `Object.prototype` | `'__proto__'` | +| `resolveDiscoveryEnvironment('constructor')` | the `Object` function | `'development'` | +| `resolveDiscoveryEnvironment('__proto__')` | `Object.prototype` | `'development'` | +| `pluralToSingular('constructor')` | the `Object` function | `'constructor'` | +| `singularToPlural('__proto__')` | `Object.prototype` | `'__proto__'` | + +Each function's declared refusal value is what it now answers — the same value +each already gave for an ordinary unknown word such as `nope`. ⛔ No new +fallback was invented. `resolveDiscoveryEnvironment` is the sharpest case: its +own docblock promises "a value guaranteed to satisfy +`DiscoveryEnvironmentSchema`", and for `constructor` it returned a `Function`. + +⚠️ **Why `minor` and not `patch`.** The level is carried by this change's +declared contract-review status, ⛔ not by a widening — the guard only NARROWS. +An off-vocabulary key that previously resolved an inherited member now gets each +function's own declared refusal value, and nothing that answered before answers +differently. Nothing in the declared vocabulary moves: every canonical operator, +every `EnvironmentType` bucket, both operator shorthands and every manifest +collection spelling answers byte-identically to before, and the only inputs +whose answer changes are the four prototype-member spellings above, which no +signature ever admitted. + +The guard is the `Object.prototype.hasOwnProperty.call(table, key) && table[key]` +shape already landed in `src/data/type-compat.ts`, and carries that site's two +recorded rejections: ⛔ not a null-prototype table (it does not type-check +against the `Record` annotation, and the spelling that does compile silently +costs the exhaustiveness check), and ⛔ not a list of prototype member names +(which the next prototype member defeats). diff --git a/packages/spec/src/api/discovery.test.ts b/packages/spec/src/api/discovery.test.ts index d4912010db..91a9b15c3b 100644 --- a/packages/spec/src/api/discovery.test.ts +++ b/packages/spec/src/api/discovery.test.ts @@ -12,6 +12,7 @@ import { ServiceSelfInfoSchema, readServiceSelfInfo, resolveDiscoveryEnvironment, + DiscoveryEnvironmentSchema, SERVICE_SELF_INFO_KEY, type DiscoveryResponse, type ApiRoutes, @@ -1366,3 +1367,55 @@ describe('[#6287] the fold table is total over EnvironmentType', () => { expect(Object.keys(missingTrial)).toHaveLength(6); }); }); + +/** + * The `Object.prototype` fall-through pin for `resolveDiscoveryEnvironment`. + * + * Its POPULATION is the point. Every other assertion on this fold above + * iterates the declared `EnvironmentTypeSchema` buckets, the two operator + * shorthands and a handful of ordinary typos — precisely the population that + * behaves — which is why the site sat green while + * `resolveDiscoveryEnvironment('constructor')` returned the `Object` FUNCTION. + * + * `raw` is uncontrolled by construction: the docblock names it as + * `process.env.NODE_ENV`, an arbitrary operator string. + * + * The contract this asserts is the function's OWN `@returns` text, verbatim: + * "a value guaranteed to satisfy {@link DiscoveryEnvironmentSchema}". So the + * assertion is a full `safeParse` against that schema, not a `typeof` check — + * the guarantee is about the VALUE, and settling for less would delete the + * coverage the sentence claims. + */ +describe('resolveDiscoveryEnvironment — Object.prototype fall-through', () => { + // Fixed at five: the three prototype methods, the assignment-shaped one, and + // a plain unknown word that names nothing at all. Four is not four-fifths of + // this pin. `toString` / `valueOf` are quiet here only by the accident that + // `spelling` is lower-cased first — they stay in the population because a + // guard that relied on that accident is exactly what this fix refuses. + const POPULATION = ['constructor', 'toString', 'valueOf', '__proto__', 'nope'] as const; + + it('folds the real taxonomy (lit control — the pin is not vacuous)', () => { + expect(resolveDiscoveryEnvironment('production')).toBe('production'); + expect(resolveDiscoveryEnvironment('prod')).toBe('production'); + expect(resolveDiscoveryEnvironment('staging')).toBe('sandbox'); + }); + + it.each(POPULATION)('%s answers a value that satisfies the declared schema', (word) => { + // What the defect produced was a `function` (and an `object` for + // `__proto__`) out of a signature that declares `DiscoveryEnvironment` — + // and out of a docblock that GUARANTEES this parse. + const answer = resolveDiscoveryEnvironment(word); + expect(typeof answer).toBe('string'); + const parsed = DiscoveryEnvironmentSchema.safeParse(answer); + expect(parsed.success, `${word} -> ${String(answer)}`).toBe(true); + }); + + it("refuses each probe with this function's own declared refusal value", () => { + // `'development'` is the trailing `return` of the function itself — the + // answer `qa`, `uat` or a typo already gets, and the one that stops a guess + // claiming `production`. ⛔ Not a value invented for the fix. + for (const word of POPULATION) { + expect(resolveDiscoveryEnvironment(word), word).toBe('development'); + } + }); +}); diff --git a/packages/spec/src/api/discovery.zod.ts b/packages/spec/src/api/discovery.zod.ts index f74688572f..1f32d5efe1 100644 --- a/packages/spec/src/api/discovery.zod.ts +++ b/packages/spec/src/api/discovery.zod.ts @@ -637,7 +637,48 @@ export function resolveDiscoveryEnvironment(raw?: string | null): DiscoveryEnvir // bucket cannot reach this line by being forgotten. Keep it: `NODE_ENV` is an // arbitrary operator string, so "anything else" is a real input class, and // degrading it to `development` is what stops a guess claiming `production`. - return NODE_ENV_TO_DISCOVERY_ENVIRONMENT[spelling] ?? 'development'; + // + // Own-property guard. The table is a plain object literal, so a bare index + // resolves `Object.prototype`'s members for an off-taxonomy `spelling`: + // `constructor` handed the `Object` FUNCTION, and `__proto__` + // `Object.prototype` itself, out of a signature that declares + // `DiscoveryEnvironment` — and out of the `@returns` above, which promises + // verbatim "a value guaranteed to satisfy {@link DiscoveryEnvironmentSchema}". + // The `??` never fires on those, because the inherited member is truthy. + // `raw` is uncontrolled by construction — it is an arbitrary operator + // `NODE_ENV` string. (`toString` / `valueOf` are quiet here only by the + // accident that `spelling` is lower-cased first; a guard that named words + // would not survive the next prototype member.) + // + // The refusal value is this function's own declared one, `'development'` — + // the same answer `qa`, `uat` or a typo already gets. The guard only narrows: + // every declared bucket and operator shorthand is an own key. + // + // ⛔ Not a null-prototype table: `src/data/type-compat.ts` records the + // measurement — a `__proto__: null` object literal does not type-check + // against the `Record<…>` annotation (TS2353), and the + // `Object.assign(Object.create(null), …)` spelling that does compile silently + // COSTS whatever check the ANNOTATION carries: `Object.create(null)` is + // `any`, and `Object.assign`'s `any & U` result is assignable to anything. + // + // Here that annotation is the outer `Readonly>`, so what the spelling would cost is its VALUE + // check — an index signature carries no key exhaustiveness to lose. Measured + // under this package's `tsconfig.json`: a bogus `dev: 'nope'` reports TS2322 + // as a literal and is silent under `Object.assign`. + // + // ⚠️ It is NOT the #6287 `satisfies Record` gate above that would be lost. `satisfies` applies + // to the literal, not to the assignment, so under that spelling a missing + // bucket still reports TS1360. Losing the value check silently is reason + // enough on its own. + if ( + Object.prototype.hasOwnProperty.call(NODE_ENV_TO_DISCOVERY_ENVIRONMENT, spelling) && + NODE_ENV_TO_DISCOVERY_ENVIRONMENT[spelling] + ) { + return NODE_ENV_TO_DISCOVERY_ENVIRONMENT[spelling]; + } + return 'development'; } // ============================================================================ diff --git a/packages/spec/src/meta-spelling/manifest-collection-spelling.test.ts b/packages/spec/src/meta-spelling/manifest-collection-spelling.test.ts index fdb076ee0c..b02f481ab7 100644 --- a/packages/spec/src/meta-spelling/manifest-collection-spelling.test.ts +++ b/packages/spec/src/meta-spelling/manifest-collection-spelling.test.ts @@ -64,3 +64,60 @@ describe('#8424 — widening the entry did not merge the two spelling contracts' expect(META_URL_TO_SINGULAR['seeds']).toBe('seed'); }); }); + +/** + * The `Object.prototype` fall-through pin for BOTH folds. + * + * Its POPULATION is the point. The assertions above iterate the declared + * manifest vocabulary and one ordinary unmapped word — precisely the population + * that behaves — which is why both sites sat green while + * `pluralToSingular('constructor')` returned the `Object` FUNCTION out of a + * signature that declares `string`. + * + * `key` is uncontrolled: these folds sit at the boundary where manifest + * collection fields and `/meta/:type` path segments — author- and + * client-supplied — are fed into the metadata registry. + * + * ⭐ `SINGULAR_TO_PLURAL` is built by `Object.fromEntries`, not written as an + * object literal. That changes nothing: `Object.fromEntries` returns an + * ORDINARY object, and the first assertion below is the measurement — both + * tables carry `Object.prototype` on their chain, so both take the same guard. + */ +describe('pluralToSingular / singularToPlural — Object.prototype fall-through', () => { + // Fixed at five: the three prototype methods a raw key can name, the + // assignment-shaped one, and a plain unknown word that names nothing at all. + // Four is not four-fifths of this pin. + const POPULATION = ['constructor', 'toString', 'valueOf', '__proto__', 'nope'] as const; + + it('both tables inherit from Object.prototype — the reason the guard is needed on BOTH', () => { + // The discriminating fact for the second fold: `Object.fromEntries` is not + // an object literal, and is an ordinary object all the same. + expect(Object.getPrototypeOf(PLURAL_TO_SINGULAR)).toBe(Object.prototype); + expect(Object.getPrototypeOf(SINGULAR_TO_PLURAL)).toBe(Object.prototype); + }); + + it('folds the real vocabulary in both directions (lit control — the pin is not vacuous)', () => { + expect(pluralToSingular('objects')).toBe('object'); + expect(singularToPlural('object')).toBe('objects'); + expect(pluralToSingular('sharingRules')).toBe('sharing_rule'); + expect(singularToPlural('sharing_rule')).toBe('sharingRules'); + }); + + it.each(POPULATION)('%s answers a string from both folds, never a prototype member', (word) => { + // What the defect produced was a `function` (and an `object` for + // `__proto__`) out of a signature that declares `string`. + expect(typeof pluralToSingular(word)).toBe('string'); + expect(typeof singularToPlural(word)).toBe('string'); + }); + + it("refuses each probe with each function's own declared refusal value", () => { + // Returning the input verbatim is the trailing `return` of each function — + // the answer an unmapped word already gets, and what keeps a store key from + // being manufactured for a collection that does not exist. ⛔ Not a value + // invented for the fix. + for (const word of POPULATION) { + expect(pluralToSingular(word), `pluralToSingular(${word})`).toBe(word); + expect(singularToPlural(word), `singularToPlural(${word})`).toBe(word); + } + }); +}); diff --git a/packages/spec/src/meta-spelling/manifest-collection-spelling.ts b/packages/spec/src/meta-spelling/manifest-collection-spelling.ts index 130512b943..c5bf5ecb7b 100644 --- a/packages/spec/src/meta-spelling/manifest-collection-spelling.ts +++ b/packages/spec/src/meta-spelling/manifest-collection-spelling.ts @@ -104,12 +104,48 @@ export const SINGULAR_TO_PLURAL: Record = Object.fromEntries( Object.entries(PLURAL_TO_SINGULAR).map(([plural, singular]) => [singular, plural]), ); +// ─────────────────────────────────────────────────────────────────────────── +// The own-property guard both folds below carry +// ─────────────────────────────────────────────────────────────────────────── +// +// Both tables sit on `Object.prototype`, so a bare index resolves its members +// for an off-vocabulary `key`: `constructor` handed the `Object` FUNCTION, and +// `toString` / `valueOf` their prototype methods, out of a signature that +// declares `string`. The `??` never fires on those, because the inherited +// member is truthy. `key` is uncontrolled: these folds sit at the boundary +// where manifest fields and `/meta/:type` path segments — both author- and +// client-supplied — are fed into the metadata registry. +// +// `SINGULAR_TO_PLURAL` is built by `Object.fromEntries` rather than written as +// a literal, which changes nothing here: `Object.fromEntries` returns an +// ORDINARY object, measured to carry `Object.prototype` on its chain exactly as +// `PLURAL_TO_SINGULAR` does. It is the same defect, and it takes the same fix. +// +// The refusal value is each function's own declared one — `key` returned +// verbatim, which is what an unmapped word already gets. The guard only +// narrows: every declared spelling is an own key, and `check:stack-collection-maps` +// pins that key set from the other side. +// +// ⛔ Not a null-prototype table, for the reason `src/data/type-compat.ts` +// records: a `__proto__: null` object literal does not type-check against the +// `Record<…>` annotation at all (TS2353), and the +// `Object.assign(Object.create(null), …)` spelling that does compile silently +// COSTS the annotation's exhaustiveness check. ⛔ Not a list of prototype +// member names either — a guard that names words does not survive the next +// prototype member. + /** Convert a plural manifest field name to its singular metadata type name. Returns the input unchanged if no mapping exists. */ export function pluralToSingular(key: string): string { - return PLURAL_TO_SINGULAR[key] ?? key; + if (Object.prototype.hasOwnProperty.call(PLURAL_TO_SINGULAR, key) && PLURAL_TO_SINGULAR[key]) { + return PLURAL_TO_SINGULAR[key]; + } + return key; } /** Convert a singular metadata type name to its plural manifest field name. Returns the input unchanged if no mapping exists. */ export function singularToPlural(key: string): string { - return SINGULAR_TO_PLURAL[key] ?? key; + if (Object.prototype.hasOwnProperty.call(SINGULAR_TO_PLURAL, key) && SINGULAR_TO_PLURAL[key]) { + return SINGULAR_TO_PLURAL[key]; + } + return key; } diff --git a/packages/spec/src/ui/view-filter-operator-prototype-fallthrough.test.ts b/packages/spec/src/ui/view-filter-operator-prototype-fallthrough.test.ts new file mode 100644 index 0000000000..cb72a13d32 --- /dev/null +++ b/packages/spec/src/ui/view-filter-operator-prototype-fallthrough.test.ts @@ -0,0 +1,61 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * The `Object.prototype` fall-through pin for `normalizeFilterOperator`. + * + * Its POPULATION is the point. Every other assertion on this fold in this + * package iterates the canonical operator vocabulary and the declared legacy + * aliases — precisely the population that behaves — which is why the site sat + * green while `normalizeFilterOperator('constructor')` returned the `Object` + * FUNCTION out of a signature that declares `string`. + * + * This site is the worst member of the family: it indexed the alias table + * TWICE, once raw and once lower-cased, so the case-folding accident that keeps + * `toString` / `valueOf` quiet at `canonicalizeSqlType` and + * `resolveDiscoveryEnvironment` does not exist here and all three prototype + * methods came back. + * + * `op` is uncontrolled: this fold is exported precisely so producers and + * renderers normalize STORED metadata through it, and a plain-JS producer has + * no compile-time narrowing at all. + */ + +import { describe, expect, it } from 'vitest'; +import { normalizeFilterOperator, VIEW_FILTER_OPERATORS } from './view.zod'; + +// Fixed at five: the three prototype methods a raw key can name, the +// assignment-shaped one, and a plain unknown word that names nothing at all. +// Four is not four-fifths of this pin. +const POPULATION = ['constructor', 'toString', 'valueOf', '__proto__', 'nope'] as const; + +describe('normalizeFilterOperator — Object.prototype fall-through', () => { + it('folds the real vocabulary (lit control — the pin is not vacuous)', () => { + expect(normalizeFilterOperator('eq')).toBe('equals'); + expect(normalizeFilterOperator('notIn')).toBe('not_in'); + expect(normalizeFilterOperator('equals')).toBe('equals'); + }); + + it.each(POPULATION)('%s answers a string, never a prototype member', (word) => { + // The assertion is on the SHAPE of the answer, not on which word it is: + // what the defect produced was a `function` (and an `object` for + // `__proto__`) out of a signature that declares `string`. + const answer = normalizeFilterOperator(word); + expect(typeof answer).toBe('string'); + }); + + it.each(POPULATION)('%s resolves to a canonical operator or to the input verbatim', (word) => { + const answer = normalizeFilterOperator(word); + const canonical = (VIEW_FILTER_OPERATORS as readonly string[]).includes(answer); + expect(canonical || answer === word, `got ${JSON.stringify(answer)}`).toBe(true); + }); + + it("refuses each probe with this function's own declared refusal value", () => { + // Returning the input verbatim is the trailing `return` of the function + // itself — the answer an unknown word like `nope` already gets, so the + // enum's own validation reports it as invalid. ⛔ Not a value invented for + // the fix. + for (const word of POPULATION) { + expect(normalizeFilterOperator(word), word).toBe(word); + } + }); +}); diff --git a/packages/spec/src/ui/view.zod.ts b/packages/spec/src/ui/view.zod.ts index 125d3059ba..1d1cfd0bef 100644 --- a/packages/spec/src/ui/view.zod.ts +++ b/packages/spec/src/ui/view.zod.ts @@ -329,7 +329,48 @@ export const VIEW_FILTER_OPERATOR_ALIASES: Record = export function normalizeFilterOperator(op: unknown): string { if (typeof op !== 'string') return op as string; if ((VIEW_FILTER_OPERATORS as readonly string[]).includes(op)) return op; - return VIEW_FILTER_OPERATOR_ALIASES[op] ?? VIEW_FILTER_OPERATOR_ALIASES[op.toLowerCase()] ?? op; + // Own-property guard on BOTH halves. The alias table is a plain object + // literal, so a bare index resolves `Object.prototype`'s members for an + // off-vocabulary `op`: `constructor` handed the `Object` FUNCTION, and + // `toString` / `valueOf` their prototype methods, out of a signature that + // declares `string`. `op` is uncontrolled here — it arrives from stored + // metadata and from plain-JS producers, which have no compile-time narrowing + // at all. + // + // This site is the worst member of the family because it indexes the table + // TWICE, once raw and once lower-cased: the case-folding accident that keeps + // `toString` / `valueOf` quiet at the sibling sites (`canonicalizeSqlType`, + // `resolveDiscoveryEnvironment`) does not exist here, so all three prototype + // methods come back. + // + // The refusal value is this function's own declared one — `op` returned + // verbatim, the trailing `return`, which is exactly what an unknown word like + // `nope` already gets, so the enum's own validation reports it as invalid. + // The guard only narrows: every alias that answered before is an own key. + // + // ⛔ Not a null-prototype table, for the reason `src/data/type-compat.ts` + // records: a `__proto__: null` object literal does not type-check against the + // `Record<…>` annotation at all (TS2353), and the + // `Object.assign(Object.create(null), …)` spelling that does compile silently + // COSTS the annotation's VALUE check — `Record` + // is an index signature, so it never carried a key-exhaustiveness check to + // lose. Measured under this package's `tsconfig.json`: a bogus `ne: 'nope'` + // reports TS2322 as a literal and is silent under `Object.assign`. A quiet + // failure is worse than a loud one. + // + // ⛔ Not a list of prototype member names either — a guard that names words + // does not survive the next prototype member. + if (Object.prototype.hasOwnProperty.call(VIEW_FILTER_OPERATOR_ALIASES, op) && VIEW_FILTER_OPERATOR_ALIASES[op]) { + return VIEW_FILTER_OPERATOR_ALIASES[op]; + } + const lowered = op.toLowerCase(); + if ( + Object.prototype.hasOwnProperty.call(VIEW_FILTER_OPERATOR_ALIASES, lowered) && + VIEW_FILTER_OPERATOR_ALIASES[lowered] + ) { + return VIEW_FILTER_OPERATOR_ALIASES[lowered]; + } + return op; } // ───────────────────────────────────────────────────────────────────────────