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-69651] CSP compatibility for ScriptApproval #582

Merged
merged 1 commit into from
Oct 14, 2024

Conversation

basil
Copy link
Member

@basil basil commented Oct 11, 2024

Use the CSP-compliant <st:bind> as in jenkinsci/warnings-ng-plugin#1859 and extract event handlers to a separate JavaScript adjunct.

Testing done

Regression testing (ATH): https://ci.jenkins.io/job/Core/job/acceptance-test-harness/job/PR-1781/4/
Regression testing (PCT): https://ci.jenkins.io/job/Tools/job/bom/job/PR-3734/4/

Verified in https://ci.jenkins.io/job/Core/job/acceptance-test-harness/job/PR-1749/11/ and https://ci.jenkins.io/job/Core/job/acceptance-test-harness/job/PR-1743/18/ that this resolves the following ATH failures when running in CSP mode (both report-only, and full CSP mode):

  • plugins.GroovyPluginTest#run_system_groovy_from_file
  • plugins.JobDslPluginTest#should_use_grooy_sandbox_no_whitelisted_content
  • plugins.JobDslPluginTest#should_use_script_approval
  • plugins.ScriptSecurityPluginTest#pipelineNeedsApproval
  • plugins.ScriptSecurityPluginTest#pipelineSignatureNeedsApproval
  • plugins.ScriptSecurityPluginTest#scriptNeedsApproval
  • plugins.ScriptSecurityPluginTest#signatureNeedsApproval

@basil basil added the internal label Oct 11, 2024
basil added a commit to basil/acceptance-test-harness that referenced this pull request Oct 11, 2024
basil added a commit to basil/bom that referenced this pull request Oct 11, 2024
basil added a commit to basil/acceptance-test-harness that referenced this pull request Oct 11, 2024
basil added a commit to basil/acceptance-test-harness that referenced this pull request Oct 11, 2024
basil added a commit to basil/acceptance-test-harness that referenced this pull request Oct 11, 2024
basil added a commit to basil/acceptance-test-harness that referenced this pull request Oct 11, 2024
basil added a commit to basil/acceptance-test-harness that referenced this pull request Oct 11, 2024
basil added a commit to basil/bom that referenced this pull request Oct 11, 2024
@@ -0,0 +1,236 @@
function hideScript(hash) {
Copy link
Member Author

@basil basil Oct 11, 2024

Choose a reason for hiding this comment

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

Diff from original:

@@ -1,4 +1,3 @@
-var mgr = <st:bind value="${it}"/>;
 function hideScript(hash) {
     document.getElementById('ps-' + hash).remove();
 }
@@ -161,8 +145,92 @@
     });
 }
 
-window.addEventListener("load", function(){
+document.addEventListener('DOMContentLoaded', function() {
     mgr.getClasspathRenderInfo(function(r) {
         renderClasspaths(r);
     });
+
+    const approveScriptButtons = document.querySelectorAll('.ps-context button.approve');
+    approveScriptButtons.forEach(function (button) {
+        button.addEventListener('click', function () {
+            approveScript(button.dataset.hash);
+        });
+    });
+
+    const denyScriptButtons = document.querySelectorAll('.ps-context button.deny');
+    denyScriptButtons.forEach(function (button) {
+        button.addEventListener('click', function () {
+            denyScript(button.dataset.hash);
+        });
+    });
+
+    const deprecatedApprovedScriptsClearButton = document.querySelector('#deprecated-approvedScripts-clear button');
+    if (deprecatedApprovedScriptsClearButton) {
+        deprecatedApprovedScriptsClearButton.addEventListener('click', function () {
+            if (confirm('Really delete all deprecated approvals? Any existing scripts will need to be requeued and reapproved.')) {
+                mgr.clearDeprecatedApprovedScripts();
+                document.getElementById('deprecated-approvedScripts-clear').style.display = 'none';
+            }
+        });
+    }
+
+    const approvedScriptsClearButton = document.querySelector('#approvedScripts-clear button');
+    approvedScriptsClearButton.addEventListener('click', function () {
+        if (confirm('Really delete all approvals? Any existing scripts will need to be requeued and reapproved.')) {
+            mgr.clearApprovedScripts();
+        }
+    });
+
+    const approveSignatureButtons = document.querySelectorAll('.s-context button.approve');
+    approveSignatureButtons.forEach(function (button) {
+        button.addEventListener('click', function () {
+            approveSignature(button.dataset.signature, button.dataset.hash);
+        });
+    });
+
+    const aclApproveSignatureButtons = document.querySelectorAll('.s-context button.acl-approve');
+    aclApproveSignatureButtons.forEach(function (button) {
+        button.addEventListener('click', function () {
+            aclApproveSignature(button.dataset.signature, button.dataset.hash);
+        });
+    });
+
+    const denySignatureButtons = document.querySelectorAll('.s-context button.deny');
+    denySignatureButtons.forEach(function (button) {
+        button.addEventListener('click', function () {
+            denySignature(button.dataset.signature, button.dataset.hash);
+        });
+    });
+
+    const approvedSignaturesClearButton = document.querySelector('#approvedSignatures-clear button');
+    approvedSignaturesClearButton.addEventListener('click', function () {
+        if (confirm('Really delete all approvals? Any existing scripts will need to be rerun and signatures reapproved.')) {
+            clearApprovedSignatures();
+        }
+    });
+
+    const dangerousApprovedSignaturesClearButton = document.querySelector('#dangerousApprovedSignatures-clear button');
+    if (dangerousApprovedSignaturesClearButton) {
+        dangerousApprovedSignaturesClearButton.addEventListener('click', function () {
+            clearDangerousApprovedSignatures();
+        });
+    }
+
+    const deprecatedApprovedClasspathsClearButton = document.querySelector('#deprecated-approvedClasspaths-clear-btn button');
+    if (deprecatedApprovedClasspathsClearButton) {
+        deprecatedApprovedClasspathsClearButton.addEventListener('click', function () {
+            if (confirm('This will be scheduled on a background thread. You can follow the progress in the system log')) {
+                mgr.convertDeprecatedApprovedClasspathEntries();
+                document.getElementById('deprecated-approvedClasspaths-clear-btn').style.display = 'none';
+                document.getElementById('deprecated-approvedClasspaths-clear-spinner').style.display = '';
+            }
+        });
+    }
+
+    const approvedClasspathEntriesClearButton = document.querySelector('#approvedClasspathEntries-clear button');
+    approvedClasspathEntriesClearButton.addEventListener('click', function () {
+        if (confirm('Really delete all approvals? Any existing scripts using a classpath will need to be rerun and entries reapproved.')) {
+            clearApprovedClasspathEntries();
+        }
+    });
 });

Copy link
Member Author

@basil basil Oct 11, 2024

Choose a reason for hiding this comment

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

Explanation of changes:

renderClasspaths(r);
});

const approveScriptButtons = document.querySelectorAll('.ps-context button.approve');
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 element is always displayed, so no need for an if statement on the next line.

});
});

const denyScriptButtons = document.querySelectorAll('.ps-context button.deny');
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 element is always displayed, so no need for an if statement on the next line.

});

const deprecatedApprovedScriptsClearButton = document.querySelector('#deprecated-approvedScripts-clear button');
if (deprecatedApprovedScriptsClearButton) {
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 element might not be displayed, so an if statement is needed.

});
}

const approvedScriptsClearButton = document.querySelector('#approvedScripts-clear button');
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 element is always displayed, so no need for an if statement on the next line.

});
});

const approvedSignaturesClearButton = document.querySelector('#approvedSignatures-clear button');
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 element is always displayed, so no need for an if statement on the next line.

}
});

const dangerousApprovedSignaturesClearButton = document.querySelector('#dangerousApprovedSignatures-clear button');
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 element is always displayed, so no need for an if statement on the next line.

}

const deprecatedApprovedClasspathsClearButton = document.querySelector('#deprecated-approvedClasspaths-clear-btn button');
if (deprecatedApprovedClasspathsClearButton) {
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 element might not be displayed, so an if statement is needed.

});
}

const approvedClasspathEntriesClearButton = document.querySelector('#approvedClasspathEntries-clear button');
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 element is always displayed, so no need for an if statement on the next line.

basil added a commit to basil/acceptance-test-harness that referenced this pull request Oct 11, 2024
basil added a commit to basil/bom that referenced this pull request Oct 11, 2024
basil added a commit to basil/acceptance-test-harness that referenced this pull request Oct 11, 2024
basil added a commit to basil/acceptance-test-harness that referenced this pull request Oct 11, 2024
@basil basil marked this pull request as ready for review October 11, 2024 21:58
@basil basil requested a review from a team as a code owner October 11, 2024 21:58
@basil
Copy link
Member Author

basil commented Oct 11, 2024

@yaroslavafenkin Are you interested in helping me manually test the remaining cases that aren't covered by automation in the Testing Done section? If not, I will try to get to it sometime next week.

@yaroslavafenkin
Copy link
Contributor

@yaroslavafenkin Are you interested in helping me manually test the remaining cases that aren't covered by automation in the Testing Done section? If not, I will try to get to it sometime next week.

Yeah, I'm interested. That would be on Monday though, I hope that's not too late.

@basil
Copy link
Member Author

basil commented Oct 11, 2024

Sure, that sounds great. Let's pick up where we left off on Monday.

basil added a commit to basil/bom that referenced this pull request Oct 12, 2024
Copy link
Contributor

@yaroslavafenkin yaroslavafenkin left a comment

Choose a reason for hiding this comment

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

Works well for me in local testing, no violations appear in the CSP report after I've clicked all the buttons I know of on the script approval page. Everything behaves correctly AFAICT.

@basil basil merged commit 4778ca8 into jenkinsci:master Oct 14, 2024
17 checks passed
@basil basil deleted the JENKINS-69651 branch October 14, 2024 14:37
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