endojs / endo

Endo is a distributed secure JavaScript sandbox, based on SES
Apache License 2.0
819 stars 71 forks source link

feat(ses): fix undefined AsyncGenerator prototype on Hermes #2220

Closed leotm closed 5 months ago

leotm commented 5 months ago

closes: #XXXX refs: https://github.com/endojs/endo/issues/1891

Description

Context: Running SES on React Native on Android (Hermes)

The AsyncGenerator prototype const AsyncGeneratorPrototype = AsyncGenerator.prototype is undefined on Hermes but we could get round this by redefining the AsyncIteratorPrototype instead we could also keep the original definition, but fallingback to this one

This is throwing an error calling addIntrinsics(getAnonymousIntrinsics())

at const AsyncIteratorPrototype= getPrototypeOf(AsyncGeneratorPrototype); // TypeError: Cannot convert undefined value to object, js engine: hermes

Fixed in a repro here: https://github.com/leotm/RN07117SES/commit/cb44efd88ebbc9cea5450cf114ef1e93ee2996ff (ignoring the redefinition of AsyncFunctionInstance for now, which is coming as separate pr part2)

TODO: fix failing test

Alternatives considered

Security Considerations

Scaling Considerations

Documentation Considerations

Testing Considerations

Compatibility Considerations

Upgrade Considerations

leotm commented 5 months ago

TODO: fix failing test

  Uncaught exception in test/test-static-module-record.js

  TypeError: Unexpected intrinsic intrinsics.%AsyncGeneratorPrototype%.__proto__ at %AsyncIteratorPrototype%

  TypeError: Unexpected intrinsic intrinsics.%AsyncGeneratorPrototype%.__proto__ at %AsyncIteratorPrototype%
    at visitPrototype (file:///Users/leo/Documents/GitHub/endo/packages/ses/src/permits-intrinsics.js:160:11)
    at visitProperties (file:///Users/leo/Documents/GitHub/endo/packages/ses/src/permits-intrinsics.js:272:5)
    at isAllowedPropertyValue (file:///Users/leo/Documents/GitHub/endo/packages/ses/src/permits-intrinsics.js:170:7)
    at isAllowedProperty (file:///Users/leo/Documents/GitHub/endo/packages/ses/src/permits-intrinsics.js:233:14)
    at visitProperties (file:///Users/leo/Documents/GitHub/endo/packages/ses/src/permits-intrinsics.js:283:26)
    at whitelistIntrinsics (file:///Users/leo/Documents/GitHub/endo/packages/ses/src/permits-intrinsics.js:323:5)
    at repairIntrinsics (file:///Users/leo/Documents/GitHub/endo/packages/ses/src/lockdown.js:348:3)
    at globalThis.lockdown (file:///Users/leo/Documents/GitHub/endo/packages/ses/src/lockdown-shim.js:16:28)
    at file:///Users/leo/Documents/GitHub/endo/packages/static-module-record/test/lockdown.js:3:1
    at ModuleJob.run (node:internal/modules/esm/module_job:194:25)

  ✘ test/test-static-module-record.js exited with a non-zero exit code: 1

caused by importing lockdown (required otherwise Compartment is undefined) https://github.com/endojs/endo/blob/dc486dcb678fa8472d4ee87ae4abd19099987170/packages/static-module-record/test/test-static-module-record.js#L7

after our redefinition of the AsyncIteratorPrototype

https://github.com/endojs/endo/blob/dc486dcb678fa8472d4ee87ae4abd19099987170/packages/ses/src/get-anonymous-intrinsics.js#L125

throwing at visitPrototype

https://github.com/endojs/endo/blob/dc486dcb678fa8472d4ee87ae4abd19099987170/packages/ses/src/permits-intrinsics.js#L160

unable to clean the new [[prototype]] we've introduced

Unexpected intrinsic intrinsics.%AsyncGeneratorPrototype%.__proto__ at %AsyncIteratorPrototype%

the error is confusing because intrinsics.%AsyncGeneratorPrototype%.__proto__ seems expected, which outputs [object Object] in other engines using (async function* () {}.constructor).prototype.prototype.__proto__

and we can't simply skip throwing in this case, otherwise subsequent related test breaks Uncaught exception in test/test-anticipate-async-iterator-helpers-shimmed.js ✘ test/test-anticipate-async-iterator-helpers-shimmed.js

so perhaps redefining AsyncGeneratorPrototype instead may yield a better result if possible or an alternative redefinition of AsyncIteratorPrototype

or a hermes-flavour shim that excludes AsyncGeneratorPrototype and relevant parts entirely

mhofman commented 5 months ago

I think I'm getting confused by what the prototype chains of iterators/generators are in Hermes, and how they differ from the spec. The permits list really is a representation of the intrinsics graph a standards compliant engine is supposed to setup. Any way to summarize the difference?

leotm commented 5 months ago

I think I'm getting confused by what the prototype chains of iterators/generators are in Hermes, and how they differ from the spec. The permits list really is a representation of the intrinsics graph a standards compliant engine is supposed to setup. Any way to summarize the difference?

makes sense ^ i misread the SES permit TypeError: Unexpected intrinsic intrinsics.%AsyncGeneratorPrototype%.__proto__ at %AsyncIteratorPrototype% thinking it's a false positive since

// unexpected %AsyncGeneratorPrototype%.__proto__
const AsyncGeneratorFunction = async function* () {}.constructor;
const AsyncGenerator = AsyncGeneratorFunction.prototype;
const AsyncGeneratorPrototype = AsyncGenerator.prototype;
const AsyncGeneratorPrototype__proto__ = AsyncGeneratorPrototype.__proto__
// expected [object Object] in engines supporting async generators

the Hermes async (arrow)fn/generator/iterator is bit confusing w vague/old features list, but w the testsuite confirmed

async functions supported async arrow functions unsupported transformed by babel async generators/iterators unsupported (will be added to Static Hermes) it looks like babel differs from the spec and was hinted at before so @babel/plugin-transform-async-to-generator may need an update then bumping in react-native-babel-preset and metro-react-native-babel-preset if backported

some errors in the ses shim are surpressed so raised this here

also Metro errors on RN 71.15 when bundling async functions/generators are unsupported but not in later versions

so think should close this pr and chase up with babel then update here