fix(codegen,parser): isKnownEntity must not accept Object.prototype members

.changeset/known-entity-prototype-chain.md

@@ -0,0 +1,27 @@
+---
+'@ifc-lite/parser': patch
+'@ifc-lite/codegen': patch
+---
+
+Stop the generated schema registry answering for `Object.prototype` members.
+
+`SCHEMA_REGISTRY.entities` is a plain object literal, so `in` and `obj[key]`
+both reach the prototype chain. `getEntityMetadata('constructor')` returned
+the `Object` constructor. Two exported guards were wrong as a result:
+
+- `isInstantiable('constructor')` was `true`. Its own docblock says it exists
+  to stop authoring code writing an abstract class into an exported file.
+- `normalizeIfcTypeName` returned the string `"Object"` for `constructor`, and
+  `undefined` for `__proto__` from a signature declaring `string`.
+
+`isKnownType('constructor')` was already `false` and is unchanged. It is worth
+naming, because the guard that reads as looser was the one answering correctly,
+and the guard documented as the strict authoring boundary was the one letting
+it through.
+
+`isKnownEntity` had the same defect and now delegates to `getEntityMetadata`
+rather than repeating the lookup.
+
+The same generator emits a second registry with the same defect, also fixed:
+`getTypeId('constructor')` returned the `Object` constructor from a signature
+declaring `number | undefined`.

packages/codegen/src/type-ids-generator.ts

@@ -66,6 +66,7 @@ export const TYPE_NAMES: Record<number, string> = {
 export function getTypeId(name: string): number | undefined {
   // Normalize to IfcXxx format
   const normalized = normalizeTypeName(name);
+  if (!Object.prototype.hasOwnProperty.call(TYPE_IDS, normalized)) return undefined;
   return (TYPE_IDS as Record<string, number>)[normalized];
 }
 

packages/codegen/src/typescript-generator.ts

@@ -461,6 +461,15 @@ export const SCHEMA_REGISTRY: SchemaRegistry = {
     code += `    ${select.name}: [${escapedTypes.join(', ')}],\n`;
   }
 
+  // Everything from here to the end of this template literal is EMITTED TEXT,
+  // not code that runs here. Two consequences that have both bitten:
+  //
+  //  - A backtick anywhere in it closes the literal and the rest becomes real
+  //    TypeScript. That is a `tsc` failure on this package, and the only thing
+  //    that catches it, so never write one in a comment below.
+  //  - The registry is a plain object literal, so `in` and `obj[key]` both
+  //    walk the prototype chain and answer for `constructor`, `toString` and
+  //    `__proto__`. Every lookup below must be an own-property check (#3063).
   code += `  },
 };
 
@@ -470,6 +479,7 @@ export const SCHEMA_REGISTRY: SchemaRegistry = {
 export function getEntityMetadata(typeName: string): EntityMetadata | undefined {
   // Normalize to IfcXxx format
   const normalized = normalizeTypeName(typeName);
+  if (!Object.prototype.hasOwnProperty.call(SCHEMA_REGISTRY.entities, normalized)) return undefined;
   return SCHEMA_REGISTRY.entities[normalized];
 }
 
@@ -493,8 +503,7 @@ export function getInheritanceChainForEntity(typeName: string): string[] {
  * Check if a type is a known entity
  */
 export function isKnownEntity(typeName: string): boolean {
-  const normalized = normalizeTypeName(typeName);
-  return normalized in SCHEMA_REGISTRY.entities;
+  return getEntityMetadata(typeName) !== undefined;
 }
 
 /**

packages/codegen/test/typescript-generator-mapping.test.ts

@@ -300,6 +300,25 @@ describe('emitted SCHEMA_REGISTRY runtime helpers (executed, not substring-match
     expect(mod.isKnownEntity('IFCNOTATHING')).toBe(false);
   });
 
+  // The registry is a plain object literal, so `in` and `obj[key]` both reach
+  // Object.prototype. Every lookup the generator emits has to be an
+  // own-property check, and getEntityMetadata is the one that matters: it
+  // feeds isInstantiable and normalizeIfcTypeName in @ifc-lite/parser, so
+  // before #3063 `isInstantiable('constructor')` was true and
+  // `normalizeIfcTypeName('constructor')` was the string "Object".
+  //
+  // `__proto__` is the case that is not like the others: it resolves to an
+  // object rather than a function, and its `.name` is undefined, which made
+  // normalizeIfcTypeName return undefined from a signature promising string.
+  it.each(['constructor', 'toString', 'hasOwnProperty', '__proto__'])(
+    'does not treat the inherited property %s as an entity',
+    async (name) => {
+      const mod = await evalEmitted(code.schemaRegistry);
+      expect(mod.isKnownEntity(name)).toBe(false);
+      expect(mod.getEntityMetadata(name)).toBeUndefined();
+    }
+  );
+
   it('carries isAbstract through to the runtime metadata, both values', async () => {
     const mod = await evalEmitted(code.schemaRegistry);
     expect(mod.getEntityMetadata('IfcRoot')?.isAbstract).toBe(true);

packages/parser/src/ifc-schema.prototype-guard.test.ts

@@ -0,0 +1,85 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at https://mozilla.org/MPL/2.0/. */
+
+import { describe, it, expect } from 'vitest';
+import { isKnownType, isInstantiable, normalizeIfcTypeName } from './ifc-schema.js';
+import { getTypeId } from './generated/type-ids.js';
+
+/**
+ * #3063. The generated SCHEMA_REGISTRY is a plain object literal, so `in` and
+ * `obj[key]` both reach Object.prototype.
+ *
+ * This file pins the DAMAGE, not the mechanism. The mechanism is pinned in
+ * packages/codegen/test/typescript-generator-mapping.test.ts, against the text
+ * the generator emits, which is the only copy that cannot drift. What that
+ * cannot see is which of this package's exported guards the defect reached,
+ * and those are the ones callers actually hold:
+ *
+ *   isKnownType('constructor')           false   (was already false)
+ *   isInstantiable('constructor')        TRUE    <- the authoring guard
+ *   normalizeIfcTypeName('constructor')  "Object"
+ *   normalizeIfcTypeName('__proto__')    undefined, from a `: string` signature
+ *
+ * isInstantiable is the one that matters. Its own docblock says it exists so
+ * authoring code cannot write an abstract class into an exported file, and it
+ * was the weaker of the two: it said yes to `constructor` while isKnownType,
+ * the guard that reads as looser, correctly said no.
+ */
+describe('schema guards reject inherited Object.prototype names', () => {
+  // `__proto__` is deliberately in this list and is not like the others: it
+  // resolves to an object rather than a function, so its `.name` is undefined
+  // rather than a string. It is the case that turned a wrong answer into a
+  // type lie.
+  const inherited = ['constructor', 'toString', 'hasOwnProperty', '__proto__'];
+
+  it.each(inherited)('isInstantiable(%s) is false', (name) => {
+    expect(isInstantiable(name)).toBe(false);
+  });
+
+  it.each(inherited)('isKnownType(%s) is false', (name) => {
+    expect(isKnownType(name)).toBe(false);
+  });
+
+  it.each(inherited)('normalizeIfcTypeName(%s) returns the name unchanged', (name) => {
+    // Unknown names are preserved as-is, because a vendor extension is not an
+    // error. The failure being pinned is returning something ELSE: "Object"
+    // for `constructor`, or undefined for `__proto__`.
+    const result = normalizeIfcTypeName(name);
+    expect(typeof result).toBe('string');
+    expect(result).toBe(name);
+  });
+
+  // The same generator emits a second registry, and it had the same defect.
+  // The comment added to typescript-generator.ts says every lookup below it
+  // must be an own-property check, which was true of that file and read as
+  // covering the generator, so this is the sibling that claim would have hidden.
+  it.each(inherited)('getTypeId(%s) is undefined, not a function', (name) => {
+    const id = getTypeId(name);
+    // `number | undefined` is the declared return. Before the fix this handed
+    // back the Object constructor for `constructor` and Object.prototype for
+    // `__proto__`, so asserting `undefined` alone would pass for the wrong
+    // reason if the signature were ever loosened. Assert the type too.
+    expect(id).toBeUndefined();
+    expect(typeof id).not.toBe('function');
+  });
+
+  it('still resolves a real type id', () => {
+    expect(typeof getTypeId('IfcWall')).toBe('number');
+  });
+
+  // Without these the suite is satisfied by making every guard return false,
+  // which would be a worse bug than the one being fixed.
+  it('still answers for real entities', () => {
+    expect(isKnownType('IfcWall')).toBe(true);
+    expect(isInstantiable('IfcWall')).toBe(true);
+    expect(normalizeIfcTypeName('IFCWALL')).toBe('IfcWall');
+  });
+
+  it('still reports an abstract class as known but not instantiable', () => {
+    // The distinction isInstantiable exists to draw. If the fix had broken it,
+    // the inherited-name cases above would still pass.
+    expect(isKnownType('IfcRoot')).toBe(true);
+    expect(isInstantiable('IfcRoot')).toBe(false);
+  });
+});