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.

Sign up for GitHub

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

Jump to bottom

Backmerge: #3627 - Ketcher requires unsafe-eval in order to run, which contradicts content security policy best practises #6269

Merged
merged 4 commits into from
Jan 20, 2025
Merged

Backmerge: #3627 - Ketcher requires unsafe-eval in order to run, which contradicts content security policy best practises #6269

merged 4 commits into from
Jan 20, 2025

Conversation

MoustaphaCamara
Copy link
Contributor

@MoustaphaCamara MoustaphaCamara commented Jan 14, 2025 •
edited
Loading

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

Fix the issue mentioned in:

Supersedes:

This PR uses ajv-cli to pre-compile the validation schema and allow ketcher to run under restrictive CSP header.
Ajv is removed from ketcher-core dependencies as not used anymore, replaced by ajv-cli only.
Ajv persists in ketcher-react as it is used and doesn't need pre-compilation.

This change has been successfully used in production without any issues. Please consider merging this change so all projects with restrictive Content Security Policies can also benefit from this excellent piece of software. =)

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

@MoustaphaCamara
feat: pre-compile schema with ajv
2fed11e 
This change uses ajv-cli to pre-compile the validation schema to allow running Ketcher under restrictive CSP header.

Fix epam#3627
Fix epam#853
Supersedes epam#4749
@MoustaphaCamara
eslint ignore compiled schema
1eb2b13
@MoustaphaCamara
pre-compile schema on dev mode
2fa97a9
This was referenced Jan 17, 2025
Closed
Closed
@rrodionov91 rrodionov91 linked an issue Jan 17, 2025 that may be closed by this pull request
Ketcher requires unsafe-eval in order to run, which contradicts content security policy best practises #3627
Closed
@rrodionov91
Copy link
Collaborator

Hi @MoustaphaCamara
Thank you very much for this PR.

CI Build job failed
image

Coud you please check what is bad with prettier there?

@NicolasCARPi
Copy link

Coud you please check what is bad with prettier there?

TL;DR: prettier version is too old

In version 3+, prettier ignores files added to .gitignore (source 1, source 2).

The current package.json pins it to 2.x. @MoustaphaCamara will be back on Monday and work on this. Am I right to assume that you prefer a separate PR with the prettier upgrade (which might have side effects)?

@rrodionov91
Copy link
Collaborator

Coud you please check what is bad with prettier there?

TL;DR: prettier version is too old

In version 3+, prettier ignores files added to .gitignore (source 1, source 2).

The current package.json pins it to 2.x. @MoustaphaCamara will be back on Monday and work on this. Am I right to assume that you prefer a separate PR with the prettier upgrade (which might have side effects)?

I think yes, it would be prefferable to have a separate PR for that.
Maybe for current PR we can use .prettierignore or .eslintignore files? Just to speed up merge

@MoustaphaCamara
Copy link
Contributor Author

Maybe for current PR we can use .prettierignore or .eslintignore files? Just to speed up merge

Seems good to me, i've updated the ignore files for the current PR @rrodionov91 .
Thanks for your feedback

@rrodionov91 rrodionov91 merged commit f9baa14 into epam:master Jan 20, 2025
11 checks passed
@rrodionov91 rrodionov91 mentioned this pull request Jan 20, 2025
Closed
9 tasks
rrodionov91 pushed a commit that referenced this pull request Jan 20, 2025
@MoustaphaCamara @rrodionov91
#3627 - Ketcher requires unsafe-eval in order to run, which contradic…
3edab12 
…ts content security policy best practises (#6269)

This change uses ajv-cli to pre-compile the validation schema to allow running Ketcher under restrictive CSP header.
- pre-compile schema on dev mode
- prettier ignore compiled schema

(cherry picked from commit f9baa14)
@rrodionov91 rrodionov91 linked an issue Jan 20, 2025 that may be closed by this pull request
Content Security Policy - "unsafe-eval" #853
Closed
@rrodionov91 rrodionov91 changed the title #3627 - Ketcher requires unsafe-eval in order to run, which contradicts content security policy best practises Backmerge: #3627 - Ketcher requires unsafe-eval in order to run, which contradicts content security policy best practises Jan 20, 2025
lmhs pushed a commit that referenced this pull request Jan 29, 2025
@MoustaphaCamara @lmhs
#3627 - Ketcher requires unsafe-eval in order to run, which contradic…
8ec472c 
…ts content security policy best practises (#6269)

This change uses ajv-cli to pre-compile the validation schema to allow running Ketcher under restrictive CSP header.
- pre-compile schema on dev mode
- prettier ignore compiled schema
@svvald svvald mentioned this pull request Feb 24, 2025
Closed
9 tasks
@MoustaphaCamara MoustaphaCamara mentioned this pull request Mar 4, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rrodionov91 rrodionov91 rrodionov91 approved these changes

@NataliaLoginova NataliaLoginova Awaiting requested review from NataliaLoginova

@Zhirnoff Zhirnoff Awaiting requested review from Zhirnoff

Assignees

@MoustaphaCamara MoustaphaCamara

Labels
None yet
Projects
None yet
Milestone
No milestone
3 participants
@MoustaphaCamara @rrodionov91 @NicolasCARPi