From c3f7593f3c1fed85c552ca03594e8c394906bf77 Mon Sep 17 00:00:00 2001 From: Jared Vititoe Date: Fri, 30 Jan 2026 13:15:55 -0500 Subject: [PATCH] Harden CSP by removing unsafe-inline for scripts Refactored all inline event handlers (onclick, onchange, onsubmit) to use addEventListener with data-action attributes and event delegation pattern. Changes: - views/*.php: Replaced inline handlers with data-action attributes - views/admin/*.php: Same refactoring for all admin views - assets/js/dashboard.js: Added event delegation for bulk/quick action modals - assets/js/ticket.js: Added event delegation for dynamic elements - assets/js/markdown.js: Refactored toolbar button handlers - assets/js/keyboard-shortcuts.js: Refactored modal close button - SecurityHeadersMiddleware.php: Enabled strict CSP with nonces The CSP now uses script-src 'self' 'nonce-{nonce}' instead of 'unsafe-inline', significantly improving XSS protection. Co-Authored-By: Claude Opus 4.5 --- assets/js/dashboard.js | 91 +++++++-- assets/js/keyboard-shortcuts.js | 7 +- assets/js/markdown.js | 33 +++- assets/js/ticket.js | 37 +++- middleware/SecurityHeadersMiddleware.php | 9 +- views/CreateTicketView.php | 30 ++- views/DashboardView.php | 234 ++++++++++++++++++----- views/TicketView.php | 30 ++- views/admin/ApiKeysView.php | 20 +- views/admin/CustomFieldsView.php | 62 ++++-- views/admin/RecurringTicketsView.php | 67 +++++-- views/admin/TemplatesView.php | 51 +++-- views/admin/WorkflowDesignerView.php | 51 +++-- 13 files changed, 564 insertions(+), 158 deletions(-) diff --git a/assets/js/dashboard.js b/assets/js/dashboard.js index 62c7905..ef5ecdb 100644 --- a/assets/js/dashboard.js +++ b/assets/js/dashboard.js @@ -97,7 +97,7 @@ function initMobileSidebar() { ๐Ÿ  Home - @@ -105,7 +105,7 @@ function initMobileSidebar() { โž• New - @@ -136,20 +136,75 @@ document.addEventListener('DOMContentLoaded', function() { window.location.href.includes('ticket.php') || document.querySelector('.ticket-details') !== null; const isDashboard = hasTable && !isTicketPage; - + if (isDashboard) { // Dashboard-specific initialization initStatusFilter(); initTableSorting(); initSidebarFilters(); } - + // Initialize for all pages initSettingsModal(); // Force dark mode only (terminal aesthetic - no theme switching) document.documentElement.setAttribute('data-theme', 'dark'); document.body.classList.add('dark-mode'); + + // Event delegation for dynamically created modals + document.addEventListener('click', function(e) { + const target = e.target.closest('[data-action]'); + if (!target) return; + + const action = target.dataset.action; + switch (action) { + // Bulk operations + case 'perform-bulk-assign': + performBulkAssign(); + break; + case 'close-bulk-assign-modal': + closeBulkAssignModal(); + break; + case 'perform-bulk-priority': + performBulkPriority(); + break; + case 'close-bulk-priority-modal': + closeBulkPriorityModal(); + break; + case 'perform-bulk-status': + performBulkStatusChange(); + break; + case 'close-bulk-status-modal': + closeBulkStatusModal(); + break; + case 'perform-bulk-delete': + performBulkDelete(); + break; + case 'close-bulk-delete-modal': + closeBulkDeleteModal(); + break; + // Quick actions + case 'perform-quick-status': + performQuickStatusChange(target.dataset.ticketId); + break; + case 'close-quick-status-modal': + closeQuickStatusModal(); + break; + case 'perform-quick-assign': + performQuickAssign(target.dataset.ticketId); + break; + case 'close-quick-assign-modal': + closeQuickAssignModal(); + break; + // Mobile navigation + case 'open-mobile-sidebar': + if (typeof openMobileSidebar === 'function') openMobileSidebar(); + break; + case 'open-settings-modal': + if (typeof openSettingsModal === 'function') openSettingsModal(); + break; + } + }); }); function initTableSorting() { @@ -716,8 +771,8 @@ function showBulkAssignModal() {
- - + +
@@ -829,8 +884,8 @@ function showBulkPriorityModal() {
- - + +
@@ -963,14 +1018,14 @@ function showBulkStatusModal() {
- - + +
`; - + document.body.insertAdjacentHTML('beforeend', modalHtml); } @@ -1057,14 +1112,14 @@ function showBulkDeleteModal() {
- - + +
`; - + document.body.insertAdjacentHTML('beforeend', modalHtml); } @@ -1332,8 +1387,8 @@ function quickStatusChange(ticketId, currentStatus) {
- - + +
@@ -1407,8 +1462,8 @@ function quickAssign(ticketId) {
- - + +
diff --git a/assets/js/keyboard-shortcuts.js b/assets/js/keyboard-shortcuts.js index 661364d..8da2bcb 100644 --- a/assets/js/keyboard-shortcuts.js +++ b/assets/js/keyboard-shortcuts.js @@ -123,11 +123,16 @@ function showKeyboardHelp() {
- +
`; document.body.appendChild(modal); + + // Add event listener for the close button + modal.querySelector('[data-action="close-shortcuts-modal"]').addEventListener('click', function() { + modal.remove(); + }); } diff --git a/assets/js/markdown.js b/assets/js/markdown.js index b2b4061..5fb123b 100644 --- a/assets/js/markdown.js +++ b/assets/js/markdown.js @@ -356,17 +356,36 @@ function createEditorToolbar(textareaId, containerId) { const toolbar = document.createElement('div'); toolbar.className = 'editor-toolbar'; toolbar.innerHTML = ` - - - + + + - - - + + + - + `; + // Add event delegation for toolbar buttons + toolbar.addEventListener('click', function(e) { + const btn = e.target.closest('[data-toolbar-action]'); + if (!btn) return; + + const action = btn.dataset.toolbarAction; + const targetId = btn.dataset.textarea; + + switch (action) { + case 'bold': toolbarBold(targetId); break; + case 'italic': toolbarItalic(targetId); break; + case 'code': toolbarCode(targetId); break; + case 'heading': toolbarHeading(targetId); break; + case 'list': toolbarList(targetId); break; + case 'quote': toolbarQuote(targetId); break; + case 'link': toolbarLink(targetId); break; + } + }); + container.insertBefore(toolbar, container.firstChild); } diff --git a/assets/js/ticket.js b/assets/js/ticket.js index 9c070ad..7ae1006 100644 --- a/assets/js/ticket.js +++ b/assets/js/ticket.js @@ -569,7 +569,7 @@ function showTab(tabName) { // Show selected tab and activate its button document.getElementById(`${tabName}-tab`).style.display = 'block'; - document.querySelector(`[onclick="showTab('${tabName}')"]`).classList.add('active'); + document.querySelector(`.tab-btn[data-tab="${tabName}"]`).classList.add('active'); // Load attachments when tab is shown if (tabName === 'attachments') { @@ -654,7 +654,7 @@ function renderDependencies(dependencies) { ${escapeHtml(dep.title)} ${escapeHtml(dep.status)} - + `; }); @@ -956,7 +956,7 @@ function renderAttachments(attachments) {
โฌ‡ - +
`; }); @@ -1150,7 +1150,7 @@ function showMentionSuggestions(query, textarea) { let html = ''; filtered.forEach((user, index) => { const isSelected = index === 0 ? 'selected' : ''; - html += `
+ html += `
@${escapeHtml(user.username)} ${user.display_name ? `${escapeHtml(user.display_name)}` : ''}
`; @@ -1212,6 +1212,31 @@ document.addEventListener('DOMContentLoaded', function() { el.innerHTML = highlightMentions(el.innerHTML); } }); + + // Event delegation for dynamically created elements + document.addEventListener('click', function(e) { + const target = e.target.closest('[data-action]'); + if (!target) return; + + const action = target.dataset.action; + switch (action) { + case 'remove-dependency': + removeDependency(target.dataset.dependencyId); + break; + case 'delete-attachment': + deleteAttachment(target.dataset.attachmentId); + break; + case 'select-mention': + selectMention(target.dataset.username); + break; + case 'save-edit-comment': + saveEditComment(parseInt(target.dataset.commentId)); + break; + case 'cancel-edit-comment': + cancelEditComment(parseInt(target.dataset.commentId)); + break; + } + }); }); // ======================================== @@ -1251,8 +1276,8 @@ function editComment(commentId) { Markdown
- - + +
`; diff --git a/middleware/SecurityHeadersMiddleware.php b/middleware/SecurityHeadersMiddleware.php index 5be8c20..3c920ce 100644 --- a/middleware/SecurityHeadersMiddleware.php +++ b/middleware/SecurityHeadersMiddleware.php @@ -26,12 +26,9 @@ class SecurityHeadersMiddleware { $nonce = self::getNonce(); // Content Security Policy - restricts where resources can be loaded from - // Currently using 'unsafe-inline' for scripts due to legacy onclick handlers throughout views - // NOTE: Nonce infrastructure exists (getNonce method, nonce attributes in views) but is not - // enforced in CSP until all inline handlers are refactored to use addEventListener. - // TODO: Complete refactoring of inline handlers, then change to: - // script-src 'self' 'nonce-{$nonce}' (removing unsafe-inline) - header("Content-Security-Policy: default-src 'self'; script-src 'self' 'unsafe-inline'; style-src 'self' 'unsafe-inline'; img-src 'self' data:; font-src 'self'; connect-src 'self';"); + // Using nonces for scripts to prevent XSS attacks while allowing inline scripts with valid nonces + // All inline event handlers have been refactored to use addEventListener with data-action attributes + header("Content-Security-Policy: default-src 'self'; script-src 'self' 'nonce-{$nonce}'; style-src 'self' 'unsafe-inline'; img-src 'self' data:; font-src 'self'; connect-src 'self';"); // Prevent clickjacking by disallowing framing header("X-Frame-Options: DENY"); diff --git a/views/CreateTicketView.php b/views/CreateTicketView.php index 1d54aaa..e8bac56 100644 --- a/views/CreateTicketView.php +++ b/views/CreateTicketView.php @@ -77,7 +77,7 @@ $nonce = SecurityHeadersMiddleware::getNonce();
- @@ -172,7 +172,7 @@ $nonce = SecurityHeadersMiddleware::getNonce();
- @@ -230,7 +230,7 @@ $nonce = SecurityHeadersMiddleware::getNonce();
- +
@@ -303,10 +303,32 @@ $nonce = SecurityHeadersMiddleware::getNonce(); groupsContainer.style.display = 'block'; } else { groupsContainer.style.display = 'none'; - // Uncheck all group checkboxes when hiding document.querySelectorAll('.visibility-group-checkbox').forEach(cb => cb.checked = false); } } + + // Event delegation for data-action handlers + document.addEventListener('click', function(event) { + const target = event.target.closest('[data-action]'); + if (!target) return; + + const action = target.dataset.action; + if (action === 'navigate') { + window.location.href = target.dataset.url; + } + }); + + document.addEventListener('change', function(event) { + const target = event.target.closest('[data-action]'); + if (!target) return; + + const action = target.dataset.action; + if (action === 'load-template') { + loadTemplate(); + } else if (action === 'toggle-visibility-groups') { + toggleVisibilityGroups(); + } + }); diff --git a/views/DashboardView.php b/views/DashboardView.php index e15c86b..5712eb5 100644 --- a/views/DashboardView.php +++ b/views/DashboardView.php @@ -84,7 +84,7 @@ $nonce = SecurityHeadersMiddleware::getNonce(); ๐Ÿ‘ค
- +
๐Ÿ“‹ Templates ๐Ÿ”„ Workflow @@ -96,14 +96,14 @@ $nonce = SecurityHeadersMiddleware::getNonce();
- +