lib: move kEnumerableProperty to internal/util

[Mesteery]

No description provided.

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/42404/

[aduh95]

Can you update this to use kEnumerableProperty please:

node/lib/internal/crypto/webcrypto.js

Lines 706 to 709 in 7123a00

	subtle: {
	...ObjectGetOwnPropertyDescriptor(Crypto.prototype, 'subtle'),
	enumerable: true,
	},

[Mesteery]

Can you update this to use kEnumerableProperty please:

node/lib/internal/crypto/webcrypto.js

Lines 706 to 709 in 7123a00

	subtle: {
	...ObjectGetOwnPropertyDescriptor(Crypto.prototype, 'subtle'),
	enumerable: true,
	},

Yes... but there is also ...ObjectGetOwnPropertyDescriptor(Crypto.prototype, 'subtle') with enumerable: true.

[aduh95]

Yes... but there is also ...ObjectGetOwnPropertyDescriptor(Crypto.prototype, 'subtle') with enumerable: true.

That's the thing, we don't need ...ObjectGetOwnPropertyDescriptor(Crypto.prototype, 'subtle') if we use kEnumerableProperty.

// In userland:
Object.prototype.get = () => 'tampered with in userland';

// In core
class Crypto {
  get subtle() { return 'genuine return value' }
}
Object.defineProperty(Crypto.prototype, 'subtle', { enumerable: true });
console.log(new Crypto().subtle) // 'tampered with in userland'

To protect against that, you either define all possible properties:

// In userland:
Object.prototype.get = () => 'tampered with in userland';

// In core
class Crypto {
  get subtle() { return 'genuine return value' }
}
Object.defineProperty(Crypto.prototype, 'subtle', {
  enumerable: true,

  // Repeat the already defined properties so they don't get tampered with:
  set: undefined,
  get: Object.getOwnPropertyDescriptor(Crypto.prototype, 'subtle').get,
  configurable: true,
});
console.log(new Crypto().subtle) // 'genuine return value'

Or you use a null prototype object (that's what kEnumerableProperty is using):

// In userland:
Object.prototype.get = () => 'tampered with in userland';

// In core
class Crypto {
  get subtle() { return 'genuine return value' }
}
const kEnumerableProperty = Object.create(null);
kEnumerableProperty.enumerable = true;
Object.defineProperty(Crypto.prototype, 'subtle', kEnumerableProperty);
console.log(new Crypto().subtle) // 'genuine return value'

In the webcrypto file, because kEnumerableProperty wasn't available to me, I used the second option, but the third option makes more sense imho.

[Mesteery]

Ok, thanks for the explanation!

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/42427/

[aduh95]

I think squashing makes more sense here, the first commit is not really useful on its own, it's not like we can revert one commit but not the other.

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/42430/

[nodejs-github-bot]

Landed in 998e5ac