endojs / endo

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

feat(transforms): add SES censorship evasion function #1812

Closed boneskull closed 11 months ago

boneskull commented 11 months ago

Description

Extracts the rewriteComment function from @endo/bundle-source/src/transform.js and exposes it as transformComment in the evade-censor export. This should be considered an experimental API.

Motivation

We could use this functionality in LavaMoat itself, so extracting it into a module lighter than @endo/bundle-source seems appropriate.

The function is not exported from the main entry point, nor documented publicly, as to avoid committing to a API we may not be ready to commit to. I can change this, if desired!

Notes

Questions

kriskowal commented 11 months ago

I defer to @michaelfig or @erights regarding the comment end marker escape and why we replace // with /* style comments. We should capture the reasoning in the code!

I have no strong feelings about how to escape homoglphys.

Also worth noting that there are other evasive transforms we hope to add one day to provide better compatibility with the ecosystem, particularly transforms that break strings that have censored terms.

boneskull commented 11 months ago

@kriskowal Which unit test do you mean?

erights commented 11 months ago

why we replace // with /* style comments. We should capture the reasoning in the code!

I do not remember being aware that we changed comments this way, so I also do not remember why. If I was aware at the time, my apologies for not capturing or at least leaving more clues.

kriskowal commented 11 months ago

@kriskowal Which unit test do you mean?

There is none as such and we are relying on integration tests inside bundle-source for the effect. I’m willing to continue in that vein.

boneskull commented 11 months ago

There is none as such and we are relying on integration tests inside bundle-source for the effect. I’m willing to continue in that vein.

For posterity: this PR does include some unit tests.

boneskull commented 11 months ago

What do you think of exporting evadeCensor(source, sourceMap) => {source, sourceMap} for less public coupling?

@kriskowal

This sounds like I'd need to copy makeLocationUnmapper() and other functions from bundle-source/src/transform.js here as well.

Was the intention to rehome the entirety of bundle-source/src/transform.js into the transform pkg? Apologies if that wasn't clear. And if so, you want it all in this PR (with multiple, logical commits)?

kriskowal commented 11 months ago

What do you think of exporting evadeCensor(source, sourceMap) => {source, sourceMap} for less public coupling?

@kriskowal

This sounds like I'd need to copy makeLocationUnmapper() and other functions from bundle-source/src/transform.js here as well.

Was the intention to rehome the entirety of bundle-source/src/transform.js into the transform pkg? Apologies if that wasn't clear. And if so, you want it all in this PR (with multiple, logical commits)?

I had not thought that many steps ahead, but that would seem to be necessary in order to decouple the public interface from the encapsulated dependencies, so I’m in favor.

boneskull commented 11 months ago

this is getting closer, but I need to add some more tests. I'm not sure how unit-y I can make them, though...

boneskull commented 11 months ago

@kriskowal Please let me know if my test fixtures in @endo/transforms are overkill

boneskull commented 11 months ago

I suppose I should update README.md with API docs too

boneskull commented 11 months ago

@kriskowal OK, I've

  1. Renamed the package (I did not change the version number, however)
  2. Updated the README
  3. Narrowed the API so that only a single function is exported. Renamed it
  4. Created a couple additional adapter functions so that, ideally, swapping out the underlying AST implementation won't be so painful
  5. Accidentally discovered how to implement function overloading via JSDoc
boneskull commented 11 months ago

linefeeds. lol

boneskull commented 11 months ago

OK. Don't touch anything. @kriskowal can I be done please

boneskull commented 11 months ago

8d9d2b7 was committed from windows where I am evidently not setup properly with gpg.

kriskowal commented 11 months ago

Thanks, @boneskull. You’re green. Would you like to squash some or all of the commits or shall I command Github to do so on your behalf?

boneskull commented 11 months ago

@kriskowal Squashed