Skip to content

refactor: convert private fields to native #private#7256

Merged
B4nan merged 3 commits intomasterfrom
refactor/native-private-fields
Mar 7, 2026
Merged

refactor: convert private fields to native #private#7256
B4nan merged 3 commits intomasterfrom
refactor/native-private-fields

Conversation

@B4nan
Copy link
Member

@B4nan B4nan commented Mar 6, 2026

Summary

  • Replace TypeScript private _fieldName with native JS #fieldName for true runtime privacy enforcement
  • QueryBuilder: 27 fields converted, clone() rewritten to explicitly copy # fields (invisible to Object.keys()), added @internal getters for _joins, _tptAlias, _aliasCounter (cross-class access)
  • Collection: _count, _property, _populated#count, #property, #populated
  • EntityManager: _schema#schema
  • External callers (AbstractSqlDriver, MariaDbQueryBuilder, TransactionManager) updated to use public getters/API instead of as any casts
  • Tests updated to use public API (shouldPopulate(), _aliasCounter getter)

Notes

  • Stacked on refactor/protected-to-private
  • Native # private fields had a V8 perf issue that was fixed in V8 v11.x (Node 21+); no overhead on Node 22+
  • # fields are class-scoped (cross-instance access works within the class body), so clone() can access qb.#field directly

Test plan

  • Full test suite (5220/5220 passing)
  • yarn build passes
  • yarn lint passes (0 errors)
  • yarn tsc-check-tests passes

🤖 Generated with Claude Code

@B4nan B4nan changed the base branch from refactor/protected-to-private to master March 6, 2026 21:17
@codecov
Copy link

codecov bot commented Mar 6, 2026

Codecov Report

❌ Patch coverage is 99.94135% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 99.68%. Comparing base (17ec4f5) to head (6800f23).
⚠️ Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
packages/sql/src/query/QueryBuilderHelper.ts 98.80% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##           master    #7256    +/-   ##
========================================
  Coverage   99.68%   99.68%            
========================================
  Files         261      261            
  Lines       25068    25110    +42     
  Branches     6797     6982   +185     
========================================
+ Hits        24988    25031    +43     
+ Misses         77       76     -1     
  Partials        3        3            

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@B4nan B4nan force-pushed the refactor/native-private-fields branch from cb96d48 to 765a4cf Compare March 7, 2026 13:01
B4nan and others added 2 commits March 7, 2026 15:33
Convert TypeScript private/protected fields to native ECMAScript #private
fields across core classes for true runtime encapsulation:

- EntityManager: all private/protected fields now use #private syntax
- Collection: all internal fields and incrementCount method now #private
- QueryBuilder: consolidate ~40 scattered state fields into single
  #state: QBState<Entity> object, simplifying clone() significantly
- MikroTransformer/MikroKyselyPlugin: all internal fields now #private
- DatabaseDriver: remove unused protected logger property

Also includes:
- fix(tests): increase cache TTL in virtual entities tests (50 -> 5000ms)
- fix(oracle): use test user in healthcheck to avoid init script race
- fix(mariadb): remove unnecessary (join: any) cast in wrapPaginateSubQuery

BREAKING CHANGE: internal fields on EntityManager, Collection,
QueryBuilder, and DatabaseDriver are no longer accessible via (x as any)
casts. Use existing public API methods instead.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Convert all remaining `private` field, method, and constructor parameter
declarations across 36 files to native ECMAScript `#private` syntax.

Affected packages: core, sql, migrations, entity-generator, seeder,
libsql, oracledb.

Notable changes:
- UnitOfWork: added `unmarkAsLoaded()` public method to replace
  bracket-notation access to the now-private `loadedEntities` set
- MetadataStorage: renamed instance `#metadata` to `#metadataMap` to
  avoid TS2804 clash with static `#metadata`
- DatabaseSchema/DatabaseTable: rewrote `toJSON()` to explicitly
  reference private fields (spread excludes native #private)
- SourceFile: inlined `ReturnType<SourceFile['breakdownOfIType']>`
  since bracket access doesn't work for #private members

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@B4nan B4nan force-pushed the refactor/native-private-fields branch from 765a4cf to cb6a7fd Compare March 7, 2026 14:56

if (pk.type === 'ObjectId' && (data[pk.name] != null || data[spk.name] != null)) {
data[pk.name] = this.platform.denormalizePrimaryKey(
data[pk.name] = this.#platform.denormalizePrimaryKey(

Check warning

Code scanning / CodeQL

Prototype-polluting assignment Medium

This assignment may alter Object.prototype if a malicious '__proto__' string is injected from
library input
.

Copilot Autofix

AI 3 days ago

In general terms, the problem is that denormalizePrimaryKey writes to and deletes from a plain object (data) using computed property names (pk.name and spk.name). To prevent prototype pollution, we should ensure that these names are not dangerous special keys like "__proto__", "constructor", or "prototype" before using them as object keys. If they are dangerous, we should skip the denormalization logic for those keys.

The best minimally-invasive fix is to add a small internal guard function in EntityFactory that checks whether a property name is safe to use as an object key, and then use this check in denormalizePrimaryKey before accessing data[pk.name] and data[spk.name]. Specifically, in packages/core/src/entity/EntityFactory.ts:

  1. Add a private helper method inside EntityFactory (near other private helpers like processDiscriminatorColumn and denormalizePrimaryKey):
private isSafePropertyKey(key: string | number | symbol): boolean {
  if (typeof key !== 'string') {
    return true;
  }
  return key !== '__proto__' && key !== 'constructor' && key !== 'prototype';
}
  1. Update denormalizePrimaryKey so that before reading or writing data[pk.name] or data[spk.name], it verifies that both pk.name and spk.name are safe via isSafePropertyKey. If either is unsafe, immediately return without modifying data. This prevents any direct or indirect library/user control from ever writing to those special properties on plain objects, while leaving all existing behavior unchanged for normal keys like "id".

No changes are required in Reference.ts beyond what is already present; Reference.unwrapReference only passes through data, and the hardened denormalizePrimaryKey will now safely handle any keys it encounters.

Implementation details:

  • All edits occur in packages/core/src/entity/EntityFactory.ts, within the shown class.
  • No existing imports or external dependencies need to change; we only add a new private method and adjust an if condition.
  • The logic in denormalizePrimaryKey remains the same except for the added safety check; thus, existing functionality for legitimate entity definitions is preserved.

Suggested changeset 1
packages/core/src/entity/EntityFactory.ts

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/packages/core/src/entity/EntityFactory.ts b/packages/core/src/entity/EntityFactory.ts
--- a/packages/core/src/entity/EntityFactory.ts
+++ b/packages/core/src/entity/EntityFactory.ts
@@ -515,6 +515,13 @@
     return meta;
   }
 
+  private isSafePropertyKey(key: string | number | symbol): boolean {
+    if (typeof key !== 'string') {
+      return true;
+    }
+    return key !== '__proto__' && key !== 'constructor' && key !== 'prototype';
+  }
+
   /**
    * denormalize PK to value required by driver (e.g. ObjectId)
    */
@@ -526,7 +533,12 @@
       return;
     }
 
-    if (pk.type === 'ObjectId' && (data[pk.name] != null || data[spk.name] != null)) {
+    if (
+      pk.type === 'ObjectId' &&
+      this.isSafePropertyKey(pk.name) &&
+      this.isSafePropertyKey(spk.name) &&
+      (data[pk.name] != null || data[spk.name] != null)
+    ) {
       data[pk.name] = this.#platform.denormalizePrimaryKey(
         (data[spk.name] || data[pk.name]) as string,
       ) as EntityDataValue<T>;
EOF
@@ -515,6 +515,13 @@
return meta;
}

private isSafePropertyKey(key: string | number | symbol): boolean {
if (typeof key !== 'string') {
return true;
}
return key !== '__proto__' && key !== 'constructor' && key !== 'prototype';
}

/**
* denormalize PK to value required by driver (e.g. ObjectId)
*/
@@ -526,7 +533,12 @@
return;
}

if (pk.type === 'ObjectId' && (data[pk.name] != null || data[spk.name] != null)) {
if (
pk.type === 'ObjectId' &&
this.isSafePropertyKey(pk.name) &&
this.isSafePropertyKey(spk.name) &&
(data[pk.name] != null || data[spk.name] != null)
) {
data[pk.name] = this.#platform.denormalizePrimaryKey(
(data[spk.name] || data[pk.name]) as string,
) as EntityDataValue<T>;
Copilot is powered by AI and may make mistakes. Always verify output.
@B4nan B4nan force-pushed the refactor/native-private-fields branch 6 times, most recently from e87ae8f to 5d8a3dd Compare March 7, 2026 19:29
- Revert EntitySchema to TS private (cross-module #_meta access fails)
- Revert executeMigrations, getSchemaDiff, SeedManager.init to TS private (tests spy on them)
- Revert OracleSchemaGenerator private methods (tests call them via `as any`)
- Replace Object.create proxy in OracleSchemaGenerator with temp getForeignKeys override
- Rewrite Migrator snapshot deserialization to use setter methods instead of Object.assign
- Add internal setter methods on DatabaseTable/DatabaseSchema for snapshot loading
- Add UnitOfWork.getChangeSetComputer() for test access
- Update tests to use public APIs instead of internal property access

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@B4nan B4nan force-pushed the refactor/native-private-fields branch from 5d8a3dd to 6800f23 Compare March 7, 2026 19:50
@B4nan B4nan merged commit 34dd54d into master Mar 7, 2026
20 of 22 checks passed
@B4nan B4nan deleted the refactor/native-private-fields branch March 7, 2026 19:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant