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

Make plugin CSP compliant #952

Open
wants to merge 12 commits into
base: master
Choose a base branch
from
Open

Conversation

yaroslavafenkin
Copy link

There's an ongoing Content Security Project in Jenkins (blog post in case you want to know more) where we work towards making plugins CSP compatible. This initiative will also be followed by an implementation of a CSP enforcement mechanism in Jenkins core.

This PR targets to address all of the violations currently present in the plugin.

Note ⚠️

None of this has been tested against a real artifactory instance. I've focused on making sure the behaviour is the same before and after my change, checked if all event handlers are properly attached, the same data is passed as arguments, if objects are created properly and in correct scope.


JENKINS-74041

Commit: 61062e3

Inline event handler has been extracted to a separate JS file. Inline usages of st:bind inside JS script block have been replaced by CSP compliant version of st:bind.

Before the change
After the change

JENKINS-74042

Commit: c6e0a7d

Inline usage of st:bind inside JS script block has been replaced by CSP compliant version of st:bind.

Before the change
After the change

JENKINS-74043

Commit: ccbb39e

Inline JS script block has been moved to its own separate JS file and included via st:adjunct. Inline event handler has been extracted to a separate JS file.

There's a part that I find problematic. Script attempts to find an element with ID of useCredentialsPluginoverridingDeployerCredentials. I haven't found any usages of such ID in the repo, so script never finds one and throws an error. That error is caught by the catch block and script recursively calls itself again with 0 delay. While achieving nothing it causes a huge performance issue. I've for now set the delay for the recursive call to 1 second, and also log an error.

Before the change
After the change

JENKINS-74044

Commit: 728d8e9

Inline event handler has been extracted to a separate JS file.

Before the change
After the change

JENKINS-74045

Commit: d1f2d85

Inline usage of st:bind inside JS script block has been replaced by CSP compliant version of st:bind.

Before the change
After the change

JENKINS-74046

Commit: 1fcfdac

Inline usage of st:bind inside JS script block has been replaced by CSP compliant version of st:bind. Inline script block containing the declaration of updateDeps has been removed as no calls of the function were found in the repository.

Before the change
After the change

JENKINS-74047

Commit: 171e1bb

Inline event handler and inline script block extracted to appropriate JS files.

Before the change
After the change

JENKINS-74048

Commit: 4708587

Inline usage of st:bind inside JS script block has been replaced by CSP compliant version of st:bind.

Before the change
After the change

JENKINS-74049

Commit: 0fc0ab0

Removed the whole file as customRadioButton tag is unused. Query: https://github.com/search?q=repo%3Ajfrog%2Fjenkins-artifactory-plugin%20customRadioButton&type=code

JENKINS-74050

Commit: 9262b4f

Inline event handler and inline script block extracted to appropriate JS files. Inline st:bind removed completely as the object it created was not used and had no methods.

Before the change
After the change

JENKINS-74051

Commit: f92c8ee

Inline usages of st:bind inside JS script block have been replaced by CSP compliant version of st:bind.

Before the change
After the change

JENKINS-74052

Commit: cfde874

Inline event handlers extracted to a JS file. Conditional script blocks also extracted to a JS file preserving the logic.

Before the change
After the change



Copy link

github-actions bot commented Nov 28, 2024

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

@yaroslavafenkin
Copy link
Author

I have read the CLA Document and I hereby sign the CLA

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.

1 participant