rbuckton / reflect-metadata

Prototype for a Metadata Reflection API for ECMAScript
https://rbuckton.github.io/reflect-metadata
Apache License 2.0
3.21k stars 184 forks source link

Importable version of `reflect-metadata` that does not shim globals. #130

Closed rbuckton closed 10 months ago

rbuckton commented 3 years ago

Related:

The various APIs added to Reflect by reflect-metadata are incompatible with SES. One option is to introduce an "importable" version that can still work with TypeScript's --emitDecoratorMetadata option:

// --module esnext
// --module es2020
// --module es2015
import { Reflect } from "reflect-metadata/no-conflict";

// --module system
// --module commonjs
// --module amd
// --module umd
import { Reflect as _Reflect } from "reflect-metadata/no-conflict";
const Reflect = _Reflect; // unfortunately necessary due to how TypeScript emits import aliases to preserve live bindings.

This version of Reflect would wrap the built-in global Reflect object so that existing TypeScript emit for the __metadata helper can continue to work while allowing access to global Reflect functionality.

@erights: Unfortunately, I cannot change the default behavior of the main module without it being a major breaking change. Is it possible to detect whether code is running in an SES environment so as not to attempt mutation of the global Reflect? If that's the case, I could investigate a hybrid approach that still performed shimming when non in an SES environment, but also provided exports for the various methods.

erights commented 3 years ago

Easiest and most obviously relevant test, not at all SES specific, would be

Object.isExtensible(Reflect)

if the answer is false then you know that you can't monkey patch Reflect anyway and need to do something else. Under SES, and in the tc53 standard environment for embedded JavaScript, the answer will always be false.

rbuckton commented 3 years ago

Wasn't the problem that errors were reported after reflect-metadata had already patched globals when lockdown gets called?

Checking extensibility is easy enough, but I'm not sure it would help. I'm also not sure if just checking for the presence of lockdown and harden from SES would justify not patching by default, or if that's even feasible since that would be dependent on import order.

My biggest concern with how reflect-metadata works in an SES environment is that it allows side-channel communications due to the use of a WeakMap (i.e., you can attach metadata to a global). I wonder whether forbidding metadata mutation if the object is not extensible would be a valid mitigation, and if so, whether there is a way for an SES user to whitelist the patched API.

I'm wary that changing the current behavior to one that prevents metadata mutation on frozen objects could be a breaking change, but perhaps there is a way I could lock down the metadata API in response to a call to SES's lockdown.

I've started on a no-conflict import that avoids global mutation in any case.

erights commented 3 years ago

without it being a major breaking change

Major breaking change to what, precisely? After you introduce no-conflict and change appropriate things to use it, what would remain broken? Would those things just be broken under SES or do you have some other workaround in mind?

rbuckton commented 3 years ago

I was considering whether it would be feasible to fix the main module in some fashion rather than adding a no-conflict export (which would essentially be just a copy of the sources with some minor changes).

erights commented 3 years ago

Hi @rbuckton any progress on this?

conartist6 commented 1 year ago

I need a metadata reflection package that works today with current versions of TS and without introducing unsupported features into the language. I'll try my hand at building and publishing something.