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

#853 - Content Security Policy - "unsafe-eval" #4749

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ihuang8
Copy link
Contributor

@ihuang8 ihuang8 commented Jun 4, 2024

How the feature works? / How did you fix the issue?

(Screenshots, videos, or GIFs, if applicable)

Fixes the issue in #853. AJV does not work in a browser page with a CSP that does not allow for "unsafe-eval". Switching to use the jsonschema library as the JSON schema validator fixes this.

Check list

  • unit-tests written
  • e2e-tests written
  • documentation updated
  • PR name follows the pattern #1234 – issue name
  • branch name doesn't contain '#'
  • PR is linked with the issue
  • base branch (master or release/xx) is correct
  • task status changed to "Code review"
  • reviewers are notified about the pull request

@rrodionov91
Copy link
Collaborator

Hi @ihuang8

Thank you for creating this PR.

I'm not sure that it is a good idea to switch from ajv to jsonshema library, because ajv updates actively, supported by big companies and has 100+ millions downloads per week when jsonshema last publish was 2 years ago.

Did you consider fix according to this ajv documentation? https://ajv.js.org/security.html#content-security-policy

@ihuang8
Copy link
Contributor Author

ihuang8 commented Jun 12, 2024

@rrodionov91 I tried following the ajv documentation to pre-compile the schemas, but I'm having trouble understanding how to handle this in the use case of ajv in packages/ketcher-react/src/script/ui/component/form/form/form.jsx. Here, it looks like the form can take a custom validation format which is defined in the code, so it seems like this would need to be compiled at run-time. Do you have any suggestions for how to approach this?

@rrodionov91
Copy link
Collaborator

Hi @ihuang8
We didn't check it so deep yet. It needs some time and we have another priorities for now.
We will try to help, however it will not be fast.

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