Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature Request: local eval for SES (Secure EcmaScript) support #957

Open
Tracked by #1413
leotm opened this issue Apr 3, 2023 · 8 comments
Open
Tracked by #1413

Feature Request: local eval for SES (Secure EcmaScript) support #957

leotm opened this issue Apr 3, 2023 · 8 comments
Labels
enhancement New feature or request

Comments

@leotm
Copy link

leotm commented Apr 3, 2023

Problem

Hey there, Leo here from team LavaMoat and MetaMask 👋

i'm working with the folks at EndoJS bringing SES (formerly Agoric/SES) support to RN and metamask-mobile

which runs on JSC and V8, but not Hermes as we know

Excluded From Support

  • Local mode eval() (use and introduce local variables)
  • with statements
# RN: Android/iOS (Hermes) with SES
ERROR  TypeError: SES cannot initialize unless 'eval' is the original intrinsic 'eval',
suitable for direct-eval (dynamically scoped eval) (SES_DIRECT_EVAL), js engine: hermes
# https://user-images.githubusercontent.com/1881059/232474795-666c83e9-53aa-4df0-a8cd-a5e45277392d.png
# https://user-images.githubusercontent.com/1881059/232477719-56c92715-b72d-4c5d-9756-a972cd88c6b1.png

stacktraces ^ from the SES lockdown shim (e.g. curl -O https://npmfs.com/download/ses/0.18.4/dist/lockdown.umd.js)
also importable via yarn add ses, but needing additional Metro and Babel config for .cjs

i note potentially supporting this feature's been discussed in the past (cc @kumavis) and past convo's

is this a feature you guys looking to make happen anytime soon?
or still only accepting community contributions

Solution

Describe the solution you'd like to happen: local eval() support 🙏
Or with statements support (can be split into separate ft req)

Alternatives considered

  • init SES without our original intrinsic eval
    • then we'd run into unsupported with statements
  • facebook/hermes community contribution: local eval()
  • facebook/hermes community contribution: with statements
  • couple further options we're exploring

Additional Context

SES

https://github.com/endojs/endo/blob/master/packages/ses/error-codes/SES_DIRECT_EVAL.md

The SES Hardened JavaScript shim captures the eval function when it is initialized. The eval function it finds must be the original eval because SES uses its dynamic scope to implement its isolated eval.

If you see this error, something running before ses initialized, most likely another instance of ses, has replaced eval with something else.

React Native LavaMoat tracker

Hermes docs

hermesengine.dev/playground

# eval(1)

/tmp/hermes-input.js:1:1: warning: Direct call to eval(), but lexical scope is not supported.

Function<global>(1 params, 1 registers, 0 symbols):
Offset in debug table: source 0x0000, lexical 0x0000
    LoadConstUInt8    r0, 1
    DirectEval        r0, r0
    Ret               r0
@leotm leotm added the enhancement New feature or request label Apr 3, 2023
@leotm leotm changed the title Feature Request: local eval for React Native SES (Secure EcmaScript) support Feature Request: local eval for SES (Secure EcmaScript) support Apr 3, 2023
@tmikov
Copy link
Contributor

tmikov commented May 16, 2023

We have planned support for local eval() in one of the next major releases of Hermes. However please note that the performance will suffer greatly.

@leotm
Copy link
Author

leotm commented May 23, 2023

that's great appreciate the update ^ and noted RE the perf consideration
then once released will monitor when lands in future RN core version
(unless by then settled on alt solution that doesn't require eval or with)

@leotm
Copy link
Author

leotm commented May 23, 2023

sry @tmikov couple more questions, are there any plans on Module Harmony / Compartments / Lockdown / Harden?
thought better asking here than spamming multiple ft requests

since we're only using/wanting direct eval to build Compartments
if you'd be happier supporting Compartments rather than direct eval? (hopefully w/o the perf hit)
or alternatively to Compartment the constituent elements? like import source, virtual Module and confined Evaluators

@tmikov
Copy link
Contributor

tmikov commented May 23, 2023

@leotm to my embarrassment, I have to profess my ignorance and admit that I haven't really followed SES closely. Frankly, at this point Hermes has gaps in EcmaScript compliance that have to be dealt with first, so Compartments and others that are still in the proposal stage haven't been on our radar.

We need local eval, so we will implement it in any case, but only the strict mode version (to be precise: local eval will work in non-strict mode, but it can't introduce variables into the surrounding scope).

We have no plans to support non-strict local eval or with - unfortunately they are incompatible with our code generation strategy and bytecode instructions. It is theoretically possible to implement them on top of JS objects with the existing bytecode, but the effort would be significant and the performance truly appalling. Is with required by your use case?

Fundamentally, Hermes was designed to be a minimalistic and efficient AOT compiler. We work best when we can access all source ahead of time, when we have static visibility into the module system (for cross-module inlining), etc. Very dynamic runtime behaviors are not really compatible with our charter. Early on we decided that we can't support every use case, because then we would just be re-implementing v8, badly. In order to offer a meaningful alternative to mainstream engines, we have to focus on the use cases where we can provide an improvement (fast startup, small binary, less memory, etc). In the next major version we will also offer a dramatic perf improvement for a certain set of use cases, but not all.

I took a quick look at https://github.com/tc39/proposal-compartments. A very cursory read makes me believe that compartments require a great deal of runtime dynamic behavior. Is that really the case?

@leotm
Copy link
Author

leotm commented May 23, 2023

😄 gotcha makes sense RE the EcmaScript compliance priorities ^ still good news for local eval and thanks for clarifying the non-strict mode behaviour, yes with is required

but we have an idea (proposal of a proposal?) without with

@leotm
Copy link
Author

leotm commented May 23, 2023

I took a quick look at https://github.com/tc39/proposal-compartments. A very cursory read makes me believe that compartments require a great deal of runtime dynamic behavior. Is that really the case?

cc @erights @kriskowal will be able to answer better than i can

@tmikov
Copy link
Contributor

tmikov commented May 24, 2023

FWIW, I think we might be able to add support for with. It feels wrong to completely ignore a language feature that has been with us for many years. It also feels good to pass most of test262. Plus, the implementation is interesting and will not slow everything down. But I can't promise it is high priority.

@leotm
Copy link
Author

leotm commented May 25, 2023

many years in the deprecation 😅 but yes covering most of test262 would :feelsgood: indeed
priority noted ^ and great the mid/low effort won't slow the roadmap down too much
thx again for the earlier Hermes history/design insight, the RN EU doc was helpful too

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants