fix(clone): support cloning objects that have an own __proto__ property

[vkarpov15]

Fix #16202

Summary

I want to consider this PR carefully before merging because of potential security ramifications (cloning __proto__ is risky).

General idea: Mongoose's clone() avoids copying __proto__, which can be problematic for objects that have an own __proto__ property. However, we want to avoid the two common workarounds for proto copying: we don't want to make clone() return a null prototype object because that would be backwards breaking, and we do not want to use Object.defineProperty() in clone() because that can be slow. Hence the alternative trick of spreading a pre-existing object with an __proto__ property, which seems to do what we want: create a plain object with ordinary prototype and proto as a property much faster than using Object.defineProperty()

Examples

[Copilot]

Pull request overview

This PR updates Mongoose’s internal clone() helper to correctly clone objects that have an own __proto__ property (common when coming from BSON/JSON), while adding tests to ensure the change does not introduce prototype-pollution regressions.

Changes:

• Update cloneObject() to allow cloning an own __proto__ key by pre-seeding the destination object with an own __proto__ data property.
• Stop using specialProperties in clone() so __proto__ is no longer skipped, while still skipping constructor and prototype.
• Add a new security-focused test suite covering several __proto__-related pollution scenarios.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File	Description
lib/helpers/clone.js	Adjusts cloning behavior to preserve own __proto__ properties without prototype pollution.
test/helpers/clone.test.js	Adds regression tests asserting no Object.prototype pollution and validating __proto__ cloning behavior.

[Copilot]

ret is written to options._seen (and may have trustedSymbol copied onto it) before potentially being reassigned when __proto__ is present. Reassigning ret breaks cycle handling (seen still points at the original {}) and drops any previously-copied trustedSymbol. Initialize ret based on whether obj has an own enumerable __proto__ before calling seen.set() / copying trustedSymbol (or update seen and reapply trustedSymbol after changing ret).

[Copilot]

This test asserts out.__proto__ === null, but that can also be true if the clone accidentally sets the object's prototype to null (via the legacy __proto__ setter) rather than creating an own __proto__ data property. To ensure the intended behavior, also assert hasOwnProperty('__proto__') and that Object.getPrototypeOf(out) is still Object.prototype.

Suggested change

	assert.strictEqual(out.a, 1);
	assert.strictEqual(out.__proto__, null);
	assert.strictEqual(out.a, 1);
	// __proto__ should be an own data property with value null, not a prototype link
	assert.strictEqual(out.__proto__, null);
	assert.ok(Object.prototype.hasOwnProperty.call(out, '__proto__'));
	assert.strictEqual(Object.getPrototypeOf(out), Object.prototype);

[Copilot]

This test only checks that the input arr still has its __proto__ value and that Object.prototype wasn't polluted, but it doesn't assert anything about the clone result. Consider asserting the returned cloned array's elements have an own __proto__ property (and that Object.getPrototypeOf(cloned[i]) === Object.prototype) to validate the new behavior for arrays, not just the absence of global pollution.

Suggested change

	clone(arr);
	assert.strictEqual(Object.prototype.arrPolluted, undefined);
	assert.strictEqual(Object.prototype.arrPolluted2, undefined);
	assert.strictEqual(arr[0].__proto__.arrPolluted, 'yes');
	const cloned = clone(arr);
	// Object.prototype must not be polluted
	assert.strictEqual(Object.prototype.arrPolluted, undefined);
	assert.strictEqual(Object.prototype.arrPolluted2, undefined);
	// Original array elements keep their own __proto__ values
	assert.strictEqual(arr[0].__proto__.arrPolluted, 'yes');
	assert.strictEqual(arr[1].__proto__.arrPolluted2, 'yes');
	// Cloned array elements should have __proto__ as an own property, not via prototype chain
	assert.ok(Object.prototype.hasOwnProperty.call(cloned[0], '__proto__'));
	assert.ok(Object.prototype.hasOwnProperty.call(cloned[1], '__proto__'));
	assert.strictEqual(Object.getPrototypeOf(cloned[0]), Object.prototype);
	assert.strictEqual(Object.getPrototypeOf(cloned[1]), Object.prototype);
	assert.strictEqual(cloned[0].__proto__.arrPolluted, 'yes');
	assert.strictEqual(cloned[1].__proto__.arrPolluted2, 'yes');

[hasezoey]

Mostly looks good to me, though the Copilot suggestions should be investigated and documentation should be made why a specific way is required (as that is not obvious at a glance)
Also, just to be sure, maybe consider this for at the very least a minor version, if not a major version.

[hasezoey]

Maybe some documentation would be good here as to why this specific way is required.