endojs / endo

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

feat(compartment-mapper): Custom parser support #2304

Closed boneskull closed 4 weeks ago

boneskull commented 1 month ago

Closes: #2303

Description

This PR adds custom parser support as discussed in #2303. To support the feature:

Security Considerations

See #2303

Scaling Considerations

See #2303

Documentation Considerations

Testing Considerations

Compatibility Considerations

None

Upgrade Considerations

boneskull commented 1 month ago

@kriskowal

I like to do import, archive, and import-archive together for any feature that applies to all three, to make sure the design is sound.

Is "import-archive" the "unarchive" to the "archive" case? If so, I'm assuming that the same custom parser would need to be provided to both invocations. What happens (or should happen) if this is not true? Would we need to do something like add the parser itself to the archive?

kriskowal commented 1 month ago

Is "import-archive" the "unarchive" to the "archive" case?

Indeed.

If so, I'm assuming that the same custom parser would need to be provided to both invocations. What happens (or should happen) if this is not true? Would we need to do something like add the parser itself to the archive?

It’s good enough to require that the same custom parsers be present for makeArchive and parseArchive (and their analogues). I don’t care to fantasize about capturing the custom parser in the archive at this time, and that can be a different route if we do go that direction.

kriskowal commented 1 month ago

Analogously, makeArchive (and writeArchive etc) has to receive compatible exits with parseArchive (and importArchive etc). Coördinating out of band is expected.

boneskull commented 1 month ago

@kriskowal OK, this is ready for Proper Review. Please take a looksee

boneskull commented 1 month ago

will update NEWS.md as a last step

boneskull commented 1 month ago

@kriskowal Note: there's no explicit test for the archive use-case. I am not sure if it's needed?

boneskull commented 1 month ago

@kriskowal OK, I've cherry-picked your changes, then fixed a problem in bundle.js. This is ready for final review. Once approved, I'll squash and merge.