refactor: convert private fields to native #private#7256
Conversation
Codecov Report❌ Patch coverage is
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. 🚀 New features to boost your workflow:
|
cb96d48 to
765a4cf
Compare
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>
765a4cf to
cb6a7fd
Compare
|
|
||
| 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
Show autofix suggestion
Hide autofix suggestion
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:
- Add a private helper method inside
EntityFactory(near other private helpers likeprocessDiscriminatorColumnanddenormalizePrimaryKey):
private isSafePropertyKey(key: string | number | symbol): boolean {
if (typeof key !== 'string') {
return true;
}
return key !== '__proto__' && key !== 'constructor' && key !== 'prototype';
}- Update
denormalizePrimaryKeyso that before reading or writingdata[pk.name]ordata[spk.name], it verifies that bothpk.nameandspk.nameare safe viaisSafePropertyKey. If either is unsafe, immediately return without modifyingdata. 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
ifcondition. - The logic in
denormalizePrimaryKeyremains the same except for the added safety check; thus, existing functionality for legitimate entity definitions is preserved.
| @@ -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>; |
e87ae8f to
5d8a3dd
Compare
- 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>
5d8a3dd to
6800f23
Compare
Summary
private _fieldNamewith native JS#fieldNamefor true runtime privacy enforcementclone()rewritten to explicitly copy#fields (invisible toObject.keys()), added@internalgetters for_joins,_tptAlias,_aliasCounter(cross-class access)_count,_property,_populated→#count,#property,#populated_schema→#schemaas anycastsshouldPopulate(),_aliasCountergetter)Notes
refactor/protected-to-private#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), soclone()can accessqb.#fielddirectlyTest plan
yarn buildpassesyarn lintpasses (0 errors)yarn tsc-check-testspasses🤖 Generated with Claude Code