feat: add reflect metadata taming
Refs: #1950
Description
reflect-metadata is a widely used package in the ecosystem. It is used to polyfill an outdated ECMAScript proposal about decorators.
It modifies the global Reflect namespace object to add several methods. Although in https://github.com/rbuckton/reflect-metadata/pull/131, @rbuckton added a /no-conflict version of this package, it is impossible to upgrade every package in the ecosystem.
This PR adds an option called reflectMetadataTaming. It has 3 valid options:
none(default): Current behavior, not try to fix it and causereflect-metadatathrow.unsafe-keep-and-inherit: If the reflect-metadata methods are added before the lockdown is called, they will be preserved like they are safe primorials. They will also be inherited by any subcompartments.mutable-per-global: The Reflect namespace is an exotic object and unique per-global. Reflect-metadata can set (prior 0.2.0) or define methods as it wishes.
Example:
unsafe-keep-and-inherit, install polyfill before lockdown
import './lockdown.umd.js'
import 'https://cdnjs.cloudflare.com/ajax/libs/reflect-metadata/0.2.2/Reflect.js'
lockdown({ reflectMetadataTaming: 'keep-and-inherit', errorTaming: 'unsafe', consoleTaming: 'unsafe' })
console.log(Reflect.metadata) // function
globalThis.c = new Compartment()
console.log(c.globalThis.Reflect === Reflect) // true
unsafe-keep-and-inherit, install polyfill after lockdown
import './lockdown.umd.js'
lockdown({ reflectMetadataTaming: 'keep-and-inherit', errorTaming: 'unsafe', consoleTaming: 'unsafe' })
await import('https://cdnjs.cloudflare.com/ajax/libs/reflect-metadata/0.2.2/Reflect.js')
// Cannot define property decorate, object is not extensible
mutable-per-global, install polyfill inside subcompartment, but not the initial one
import './lockdown.umd.js'
lockdown({ reflectMetadataTaming: 'mutable-per-global', errorTaming: 'unsafe', consoleTaming: 'unsafe' })
globalThis.c = new Compartment()
console.log(c.globalThis.Reflect === Reflect) // false
var data = await fetch('https://cdnjs.cloudflare.com/ajax/libs/reflect-metadata/0.2.2/Reflect.js').then(x => x.text())
c.globalThis.eval(data)
console.log(c.globalThis.Reflect.metadata) // function
console.log(Reflect.metadata) // undefined
Security Considerations
When using keep-and-inherit, all compartment and the initial compartment shares the Metadata Registry (which is viable by Reflect[Symbol.for('@reflect-metadata:registry')]). They may exchange objects via this registry and break the membrane if there is one.
Scaling Considerations
No problem.
Documentation Considerations
If you're hitting problems with reflect-metadata, you might need this.
Testing Considerations
I have not added any tests yet. I hope I can iterate the PR design with the endo team and write the tests after the design is stable.
Compatibility Considerations
No breaking changes.
Upgrade Considerations
No breaking changes.
Update
NEWS.mdfor user-facing changes.
TODO.
Please add the word
'unsafe'somewhere in the option currently named'keep-and-inherit'
Done
Reflect will likely house getIntrinsic as well, so i think a per compartment Reflect is already quite inevitable.
@erights
For my use cases, unsafe-keep-and-inherit is enough,
permits.js remains as declarative as it is now. No imperative side effects to it based on lockdown options.
this is the best minimal change I can think of. If I patch this behavior in makeIntrinsicsCollector, I'll get more code changes than this.
If code in the start compartment wants to grant code in a constructed compartment access to its own unsafe Reflect, it does so by endowment, not by lockdown option. This recapitulates the transition we made with Math and Date, where we initially propagated the unsafe one by lockdown option, and then switched to propagation only by endowment
Can you give an example what this might be look like in the user land?
For my use cases, unsafe-keep-and-inherit is enough,
Excellent!
Can you give an example what this might be look like in the user land?
const compartment = new Compartment({
globals: { Reflect },
__options__: true, // only needed during a compatibility transition
});
Whatever Reflect is already bound to in this calling code, it passes as the Reflect endowment for the globals of the newly constructed compartment.
@erights If I read this correctly, this feature can be implemented with very little behavior contingent on a lockdown option.
- Separating %Reflect% from %SharedReflect% is safe in general. We can visit a per-compartment Reflect when we realize a future need.
- Leaving %Reflect% between repairIntrinsics and hardenIntrinsics is the normal behavior.
Adding %Reflect%.metadata to the permits might introduce a hazard in the event the Reflect is later hardened and endowed. Do we need an option?
Adding %Reflect%.metadata to the permits might introduce a hazard in the event the Reflect is later hardened and endowed. Do we need an option?
If the reflect-metadata package runs after repairIntrinsics as a vetted shim, then it runs after the permit processing, so we do not need to add anything to permits.js to accommodate all the stuff it monkey patches in to the start compartment's Reflect.
Right now, we don't support (well?) clients of ses running outside libraries as vetted shims between repairIntrinsics and hardenIntrinsics. But if we can somehow arrange that the flag leaves the start compartment's Reflect mutable even after hardenIntrinsics, then reflect-metadata can run after lockdown, still without needing to accommodate any added property in permits.js.
As you say, we do need permits.js to distinguish the start compartment's Reflect from the Reflect shared by constructed compartments.
Hi @Jack-Works , What is the status of this?
Hi @Jack-Works , What is the status of this?
Hi! I've been recently working on other projects, but this one is still on my TODO list. I'll keep it up to date after I come back to this.