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

Fixes #25032: Use Content-Security-Policy strict headers in utilities pages #5764

Conversation

clarktsiory
Copy link
Contributor

@clarktsiory clarktsiory commented Jul 5, 2024

https://issues.rudder.io/issues/25032

  • 💡 Replace the regex-based filtering of CSP headers, and use a lift directive instead : data-lift="with-enabled-csp" to include in every HTML page for which to enforce strict-CSP
  • 🔨 Migrate scripts in Archives page
  • 🔨 Migrate scripts in Event logs page
  • 🔨 Reuse the lift directive in the Healthcheck page

@clarktsiory clarktsiory requested a review from fanf July 5, 2024 13:07
@clarktsiory clarktsiory marked this pull request as draft July 5, 2024 13:08
@clarktsiory
Copy link
Contributor Author

PR updated with a new commit

@clarktsiory clarktsiory force-pushed the bug_25032/use_content_security_policy_strict_headers_in_web_pages branch from 1737e05 to c2c3241 Compare July 5, 2024 13:37
@clarktsiory clarktsiory marked this pull request as ready for review July 5, 2024 13:37
OnLoad(
JsRaw(
"""enableIfNonEmpty("%s", "%s");$("#%s").prop("disabled",true);"""
.format(archiveDateSelectId, restoreButtonId, restoreButtonId)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why keeping that format ? Interpolation wouldn't be clearer ?

Copy link
Member

@fanf fanf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does the job, and the explicit control is good during migration.
But it will be very easy to forget, so we should try the opposite in next version:

  • create a tag that disable csp checking
  • put it on content that breaks with csp checking only (ie, enabled by default).

Given there is a bunch of other things to test in that version I think we're too late to do it now, but it can be done as one of the first step of 8.3 (and then use 8.3 dev cycle to add the "without-csp" tag where it is needed)

@Normation-Quality-Assistant
Copy link
Contributor

OK, merging this PR

@Normation-Quality-Assistant Normation-Quality-Assistant merged commit 6d79135 into Normation:master Jul 8, 2024
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants