josdejong / svelte-jsoneditor

A web-based tool to view, edit, format, repair, query, transform, and validate JSON
https://jsoneditoronline.org
Other
921 stars 111 forks source link

CSP issue because the TransformModal uses `new Function` #465

Closed belaviyo closed 2 weeks ago

belaviyo commented 3 months ago

The "Transform" feature uses new Function, which may be restricted in certain cases due to CSP limitations. Could we have a custom async eval method that allows using a worker to execute the user function and return the value asynchronously?

josdejong commented 3 months ago

That is a good point. The editor has a flexible API to define one or multiple custom query languages, but right now the JavaScript query language is always included since it is the default one:

https://github.com/josdejong/svelte-jsoneditor/blob/main/src/lib/components/JSONEditor.svelte#L88

I'm not sure what would be the best way to go about this:

  1. remove the default query language and force people to configure one themselves (not user friendly)
  2. replace the default query language with an other language that doesn't use new Function. That should be a very lightweight one though, since it will always be packaged with the editor.
  3. Create a second constructor class alongside JSONEditor, which doesn't set defaults for queryLanguages and maybe some other options like pathParser. So then JSONEditor is the best to get started, and for advanced use you can use this other class.

Any thoughts?

belaviyo commented 3 months ago

I think the executeQuery should be like

function executeQuery(json, query, callback) {}

or

async function executeQuery(json, query) {}

There should be an option for developers to override this with a custom function. In many cases, new Function works well. If any issues arise, developers can create their own custom method to run user commands in a sandboxed iframe or worker and return the results asynchronously.

josdejong commented 3 months ago

Great idea to support async results in executeQuery 👍

josdejong commented 3 months ago

I'm working on a safe and very lightweight JSON query language to include as default in svelte-jsoneditor, currently in an early stage. Feedback would be welcome:

https://github.com/josdejong/jsonquery

josdejong commented 3 months ago

I was thinking about a good use case where you want to be able to run executeQuery asynchronously (after the CSP issue is solved), but I actually can't come up with one 🤔.

In my experience, executing a query runs about instantly, it maybe takes up to 1 second or so for a large 500 MB JSON document. So no need to run it in a separate worker or anything. That would even work worse: copying the document to a worker context and back would already take more time than the actual transform operation.

Do you have a concrete use case for the need for an async executeQuery? If not I suggest we don't implement async support for executeQuery.

belaviyo commented 3 months ago

Defaulting to asynchronous operations is always a good idea. With this general-purpose library, there's a chance that someone might want to override the function, and it's likely that the parser or interpreter will return results asynchronously. Like the worker method I've mentioned.

josdejong commented 3 months ago

Thanks for your feedback. An async API is indeed the most powerful and in general a good idea, however it will also require work to properly implement it (we need to show a loading icon after a delay, ensure only the results of the last request are displayed on screen when multiple requests are made shortly after each other, etc). I'm not going to implement that unless we have a concrete and valid use case for it (YAGNI).

josdejong commented 1 month ago

Since v1.0.0, the default query language used in svelte-jsoneditor doesn't use new Function anymore.

There are only two pieces of code left containing new Function:

  1. The query plugins lodashQueryLanguage and javascriptQueryLanguage.
  2. The JSON schema library Ajv used via the plugin createAjvValidator.

When you're using tree-shaking in your project, you'll not end up with the code of (1) as long as you don't use these plugins.

The same should work for Ajv (2), but Ajv doesn't yet allow tree shaking it. This is addressed via https://github.com/ajv-validator/ajv/issues/2479 but this fix is not yet published. As a workaround, you can manually add the field "sideEffects": false to the package.json of the ajv library.

josdejong commented 2 weeks ago

I'll now close this issue. The last open end will be resolved with the first next release of Ajv.