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

feat(bundle-source): Apply evasive transforms to Endo archives #2768

Merged
merged 6 commits into from
Apr 6, 2021

Conversation

kriskowal
Copy link
Member

  • chore(bundle-source): Upgrade compartment-mapper to v0.2.4
  • chore: Update yarn.lock
  • feat(bundle-source): Apply transforms to endoZipBase64
  • fix(bundle-source): Transform inner comments

refs: #2684

Description

SES censors any script or module that appears to use dynamic import.
Since the SES runtime is lightweight and risk averse, it uses a regular expression to detect this pattern, but there are many false positives, particularly apparent dynamic import within comments.
These are common especially in JSDoc comments with TypeScript imports.
The getExports and nestedEvaluate bundle formats apply a parser-based transform that rewrites comments such that SES won't censor them.

This change brings these existing transforms to bear on the individual files of an Endo compartment when using the new endoZipBase64 bundle type.
To facilitate this, a new version of the compartment mapper provides a "module transforms" hook.

The getExports and nestedEvaluate bundle formats also give Rollup a turn at transforming the input sources.
This alters the shape of the AST such that "inner comments" never occur.
An "inner comment" is a comment that neither leads nor follows non-comment code.
Such comments are common specifically in files like the conventional types.js that consists entirely of TypeScript JSDocs.

Since Endo doesn't preprocess files with Rollup, the comment rewriting transform needs to traverse this previously unexplored branch of the syntax trees.

Security Considerations

This change alters the presentation of client code to SES.
It should do so in a way that is semantically equivalent.
SES does not rely on these transformations to enforce integrity.

Documentation Considerations

This change does not require further documentation.
Users may notice in their debugger that comments with import will see Ximport instead.
The intention is to relieve the concern of SES censorship from the authors of client code, particularly third-party module authors who do not anticipate their code being executed in a SES environment.

Testing Considerations

This change introduces a new test for the inner comments.
Sufficient coverage of Endo should occur through real usage throughout the Agoric SDK, as we shift from using nestedEvaluate to endoZipBase64.

Copy link
Member

@michaelfig michaelfig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

packages/bundle-source/test/test-comment.js Outdated Show resolved Hide resolved
@kriskowal kriskowal disabled auto-merge April 6, 2021 01:22
@kriskowal kriskowal enabled auto-merge (squash) April 6, 2021 01:22
@kriskowal kriskowal merged commit e15cee8 into master Apr 6, 2021
@kriskowal kriskowal deleted the 2684-transform-endo branch April 6, 2021 01:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants