endojs / endo

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

New library based on SES : Terrar.js #1526

Closed LilaRest closed 9 months ago

LilaRest commented 1 year ago

Hi there ! :wave: I'm not sure where I can post this, so feel free to redirect me elsewhere.

I've just released a prototype SES-based library that aims to provide an efficient and permeable alternative to iframes. It is called Terrar.js :herb:

I'll appreciate any feedback and contribution. And by the way, thanks for all your work on SES.

kungfooman commented 1 year ago

So you call it Terrar.js and then uglify your source code with TypeScript first? Do you know that JSDoc exists and can simplify your build setup by using <script type="module"> for a quicker and less error-prone dev cycle?

Bad TypeScript (missing example, lines are too long, no description of parameters, no examples):

https://github.com/LilaRest/terrar/blob/82d8b3d74cea5acc0cd3602fac8df6d041424974/src/utils.ts#L1-L2

// Function to concat two arrays and remove duplicates
export const concatDistinct = (arr1: string[], arr2: string[]): string[] => [...new Set([...arr1, ...arr2])];

Good JS, because:

Good JS example:

/**
 * Function to concat two arrays and remove duplicates
 * @example concatDistinct([1, 2, 3], [2, 3, 4]) // [1, 2, 3, 4]
 * @param {string[]} arr1 - The first array to be concatenated.
 * @param {string[]} arr2 - The second array to be concatenated.
 * @returns {string[]} - The combined array with no duplicates.
 */
export const concatDistinct = (arr1, arr2) => [...new Set([...arr1, ...arr2])];
LilaRest commented 1 year ago

Hi @kungfooman ! Thanks for your time. As I mentioned earlier this a prototype / a proof-of-concept.

I've spent at most 5h on the code and my intention was just to ship as fast as possible a minimal working version to start doing some tests with it for Uiverse.io's user content isolation. (And also to see if it could interest some people)

I know about JSDoc, I'm aware that this is neither properly documented, nor tested and even not fully functional.

Any feedback that are not related to those points ?

kungfooman commented 1 year ago

Any feedback that are not related to those points ?

I would actually like to test and play around with it, but TypeScript is a deal breaker. There is no technical necessity for it, so why should I waste time and disk space installing it (+ rollup etc.) via npm.

The demo.html is not working by itself, since it requires: <script src="dist/terrar.umd.js"></script>

And even if I install everything, the debugging process is like reading obfuscated code.

TLDR: you are limiting your audience for no reason.

LilaRest commented 1 year ago

@kungfooman Typescript is in my own opinion a must-have in such a project where type uncertainty can lead to heavy consequences. I also like to develop with it, and I'm more comfortable in having control over data types.

...so why should I waste time... You 'should' nothing, only do this if you're happy to do so.

Thanks for pointing out this wrong script in demo.html, I'll update it right now with an UNPKG hosted one.

LilaRest commented 1 year ago

@kungfooman I've just updated the demo file, now it should work out of the box.

kungfooman commented 1 year ago

Typescript is in my own opinion a must-have in such a project where type uncertainty can lead to heavy consequences. I also like to develop with it, and I'm more comfortable in having control over data types.

I don't think you understand TypeScript fully. TypeScript checks JavaScript JSDoc just like TypeScript itself. You just need a jsconfig.json:

{
  "compilerOptions": {
    "target" : "es2021",
    "module" : "es2022",
    "strict" : true,
    "strictNullChecks": true,
    "checkJs": true
  }
}

And if you want control over your data types, why not use actually proper typings:

/**
 * Function to concat two arrays and remove duplicates
 * @example concatDistinct([1, 2, 3], [2, 3, 4]) // [1, 2, 3, 4]
 * @param {A[]} arr1 - The first array to be concatenated.
 * @param {B[]} arr2 - The second array to be concatenated.
 * @template A - Type of first parameter
 * @template B - Type of second parameter
 * @returns {(A|B)[]} - The combined array with no duplicates.
 */
export const concatDistinct = (arr1, arr2) => [...new Set([...arr1, ...arr2])];
// Examples with return types as comments:
const x = concatDistinct([1, 2, 3], [2, 3, 4           ]); // Type: number[]
const y = concatDistinct([1, 2, 3], ["2", "3", "4"     ]); // Type: (string | number)[]
const z = concatDistinct([1, 2, 3], ["2", "3", new Date]); // Type: (string | number | Date)[]

https://www.typescriptlang.org/play?ts=4.6.2&filetype=js#code/PQKhCgAIUgxBXAdgYwC4EsD2jKs5ZbZAQ1VwHd9iAna4gTwGdJjEATSagUwFtMA3LpDbwADgBt0JVF0ZQYAAS4APYjwlDCKUgBF0jDClQAKANoBGADSQATNYDMAXWum7ke9YAsjgJSRgwJAW1m4ekN7ykAqiNGqQAN4AgqaOAL4stOaQALSQACoAFkIAZujUBhl09Lj4AEaaRKRciE1sAHSR0bE8CQBCKek01DY5+UWQjFxaHEMMNZD1BI0yLTLtnTLq4k2QiaN59KJCmMWQpeVkMXQ8XDLUG7wSO737h8enk9OQV2q3XPfQKLcVDwaiIZjxYyJAA+vR8A3240IPFq6EQXBmtDm5HQqAKkEQ+BEEikTUYHWgwHAKlEmGoZC0FS00j0BjRaEgAF5IMYhlZKjY-JyAHxBNri9HkSAAZVuZnFbT51gVQxsvkcAG5wAFIABRVRbWSQHF4zi3UE4VBvZjEZjIm6IVCMABc4EZZGUXKW2lQrMMaDM-NCziCoS8kAjkajkF8Gv8gQOR2dBPgKP+KTd2Aq1W5zN0+n9JmCtgcIdMACIbOXrOX7NXIOXPOXo7H4-k3snjAZqGiAOaQaEptPUeGOTPgsgALy9ed9BfZRaDpZclfrtfrksgOiarZ1ia4ne7fYHQ-q1BP25ko-AQA

I've just updated the demo file, now it should work out of the box.

Thank you for updating! Last issue is that is just looks obfuscated. Modern JS is using importmaps, so you can simply write:

<html>
    <body>
        <script type="importmap">
            {
                "imports": {
                    "terrar": "./src/index.js"
                }
            }
        </script>
        <script src="https://unpkg.com/ses@0.18.2/dist/ses.umd.js"></script>
        <script src="https://unpkg.com/esprima@4.0.1/dist/esprima.js"></script>
        <script type="module">
            // Doesn't work, since it's TypeScript, convert to JSDoc with better template types like example given above
            // import * as terrar from "terrar";
            const tokens = esprima.tokenize("1 + 2");
            console.log({tokens});
            lockdown();
        </script>
    </body>
</html>
kriskowal commented 1 year ago

Thank you for taking the time to let us know you’re building on ses! It is a rare pleasure indeed for maintainers of open source to receive feedback about adoption of their work.

Please expect to wait a while for feedback from the maintainers of ses and our developer community regarding the integrity of Terrar. Our experience is that the DOM, even the shadow DOM, is difficult to confine. We’re also generally weary of including tools that were not designed for confinement in the portion of a program that is responsible for confinement. When I return from parental leave or as my colleagues have an opportunity, you can expect useful feedback.

In the interim, I’d like to ask posters on this thread to wait for our response and take general feedback for the authors of Terrar to the Terrar issue tracker. Thank you!

LilaRest commented 1 year ago

@kungfooman Thanks, I wasn't aware that Typescript could interpret JSDoc comments as typing ! If I decide to continue the development of this project, I'll definitely use this syntax which is much more readable and allows keeping the code in Vanilla.

Again, note that this project is a Proof-of-Concept and that I've just used the stack I'm the more familiar with to ship it quickly.

LilaRest commented 1 year ago

@kriskowal Thanks for your answer, glad it's a pleasure for you ^^ Of course, I'll be happy to read your feedback in the future.

As said before, Terrar.js is a proof-of-concept, partially functional. My intention through this issue is to get feedback from JS-isolation experts on the design I've chosen. To determine whether or not it could (in theory) allow JS isolation (or if there is a major flaw I haven't considered).