-
Notifications
You must be signed in to change notification settings - Fork 726
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
make plugin CSP compliant #2551
Conversation
- remove inline javascript from checkUrl - remove inline style tags and move styles to own css file
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
<html><head> | ||
<style class='anchorjs'></style><link href='https://jenkins.io/assets/bower/bootstrap/css/bootstrap.min.css' media='screen' rel='stylesheet'/> | ||
<link href='https://jenkins.io/assets/bower/bootstrap/css/bootstrap.min.css' media='screen' rel='stylesheet'/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤯 what is this doing here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know what that style could be for. The reference page works just fine without it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if its fine do you mind removing it now too?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the bootstrap is required, without it the page looks bad, I was referring to the <style class="anchorjs"></style>
, that this isn't required
Head branch was pushed to by a user without write access
Breaks:
Reverted in jenkinsci/bom#3496 |
I'm not able to reproduce the problem. |
See jenkinsci/bom#3496. |
There is no hint what could be wrong in there. |
The hint was to reproduce the problem in
Did you follow the |
OK, so the error I get is
In the surefire report I can see that the classpath contains I can workaround in configuration-as-code by sticking to the Then the |
PCT updates plugins, but it doesn't update non-plugins (like the JCasC test harness). If we want PCT to update the JCasC test harness in lockstep with the JCasC plugin, we'll need to write a hook for that in https://github.com/jenkinsci/plugin-compat-tester/tree/9eebb111439fbe8d33545288687d54103917ebeb/src/main/java/org/jenkins/tools/test/hook
This sounds perfectly fine in the short term. |
tested that after visiting the pages from CasC no things are found in CSP plugin reports
Your checklist for this pull request
🚨 Please review the guidelines for contributing to this repository.