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

[JENKINS-73955] CSP support for DockerManagementServer #1107

Merged
merged 2 commits into from
Oct 14, 2024

Conversation

basil
Copy link
Member

@basil basil commented Oct 14, 2024

Moving an inline event handler to a separate file as per https://www.jenkins.io/doc/developer/security/csp/#inline-event-handlers

Testing done

Created a Docker server with a single container and tried stopping it in the UI in restrictive CSP mode before this PR. I got a CSP violation and the attempt failed. After this PR there is no CSP violation logged and the attempt successfully stops the container.

@basil basil added the internal label Oct 14, 2024
@basil basil requested a review from a team as a code owner October 14, 2024 17:46
Comment on lines -75 to -78
public String getJsUrl(String jsName) {
return Consts.PLUGIN_JS_URL + jsName;
}

Copy link
Member Author

Choose a reason for hiding this comment

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

Dead code, as this is now served through a Stapler adjunct.

Comment on lines -13 to -17

/**
* The base URL of the plugin javascripts.
*/
public static final String PLUGIN_JS_URL = PLUGIN_URL + "js/";
Copy link
Member Author

Choose a reason for hiding this comment

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

Dead code, as this is now served through a Stapler adjunct.

Comment on lines +1 to +9
function stop(theId) {
var input = document.getElementById('stopId');
if (input != null) {
input.value = theId;
}

var form = document.getElementById('control');
form.submit();
}
Copy link
Member Author

Choose a reason for hiding this comment

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

This code previously lived in src/main/webapp/js/docker-manage.js and has been moved without alteration. It has been moved here so it can be consumed through a Stapler adjunct.

Comment on lines +11 to +18
document.addEventListener('DOMContentLoaded', () => {
const stopContainerButtons = document.querySelectorAll('#control .stop-container');
stopContainerButtons.forEach((button) =>
button.addEventListener('click', () => {
stop(button.dataset.id);
}),
);
});
Copy link
Member Author

Choose a reason for hiding this comment

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

@@ -1,9 +1,9 @@
<?jelly escape-by-default='true'?>
<j:jelly xmlns:j="jelly:core" xmlns:l="/lib/layout" xmlns:f="/lib/form"
<j:jelly xmlns:j="jelly:core" xmlns:st="jelly:stapler" xmlns:l="/lib/layout" xmlns:f="/lib/form"
Copy link
Member Author

Choose a reason for hiding this comment

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

So that the <st:> namespace can be used on line 6 below.

@@ -52,7 +52,7 @@
</j:forEach>
</td>
<td>
<input type="button" value="stop" onclick="stop('${res.id}')"></input>
<input type="button" class="stop-container" value="stop" data-id="${res.id}"></input>
Copy link
Member Author

Choose a reason for hiding this comment

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

@MarkEWaite
Copy link
Contributor

This pull requests arrives with great timing. @krisstern has asked that I create a new release of the docker plugin. I'll check these changes in my instance, merge the pull request, and deliver a new release.

@basil
Copy link
Member Author

basil commented Oct 14, 2024

Please watch out for #1103 when cutting that release.

Copy link
Contributor

@MarkEWaite MarkEWaite left a comment

Choose a reason for hiding this comment

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

Confirmed working as expected before and after the change. Thanks!

@MarkEWaite MarkEWaite merged commit 5007d28 into jenkinsci:master Oct 14, 2024
17 checks passed
@basil basil deleted the csp branch October 14, 2024 23:27
@basil basil changed the title CSP support for DockerManagementServer [JENKINS-73955] CSP support for DockerManagementServer Oct 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants